diff mbox

[1/1] P2P: Ignore RTM_NEWLINK event for the P2P Discovery Interface

Message ID CAGCGobAqTef2SzVhARXDPrOh_JTx8sg4SxjNoUoH-J9wtx4aCA@mail.gmail.com
State Accepted
Headers show

Commit Message

Jithu Jance June 16, 2014, 2:26 p.m. UTC
Looks like last email got the patch as white space corrupted. So
sending it again.

Signed-off-by: Jithu Jance <jithu@broadcom.com>
---
 src/drivers/driver_nl80211.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

    return 0;
 }
--
1.7.9.5



> Jithu Jance





On Sun, Jun 15, 2014 at 6:53 PM, Jithu Jance <jithujance@gmail.com> wrote:
> Hi Jouni/Dmitry,
>
> I applied the logic of the commit:147848ec4d26613d5a117d4b35dbc7ff98dd65d1
> to wpa_driver_nl80211_if_add function as well. Please see whether this
> patch is fine.
>
> Signed-off-by: Jithu Jance <jithu@broadcom.com>
> ---
>  src/drivers/driver_nl80211.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index 7568653..baef09d 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -10177,8 +10177,16 @@ static int wpa_driver_nl80211_if_add(void
> *priv, enum wpa_driver_if_type type,
>   if (drv->global)
>   drv->global->if_add_ifindex = ifidx;
>
> - if (ifidx > 0)
> + /*
> + * Some virtual interfaces need to process EAPOL packets and events on
> + * the parent interface. This is used mainly with hostapd.
> + */
> + if ((ifidx > 0) && (drv->hostapd ||
> +    nlmode == NL80211_IFTYPE_AP_VLAN ||
> +    nlmode == NL80211_IFTYPE_WDS ||
> +    nlmode == NL80211_IFTYPE_MONITOR)) {
>   add_ifidx(drv, ifidx);
> + }
>
>   return 0;
>  }
> --
> 1.7.9.5
>
>
>
>> Jithu Jance
>
>
>
>
>
> On Sat, Jun 14, 2014 at 7:34 PM, Jithu Jance <jithujance@gmail.com> wrote:
>> Hi Dmitry & Jouni,
>>
>> Thanks for looking into this. I think I didn't dig deep enough. I will
>> also check this and try to come up with a patch to address this.
>>
>>
>> Thanks,
>>
>>
>>
>>> Jithu Jance
>>
>>
>>
>>
>>
>> On Sat, Jun 14, 2014 at 5:18 AM, Dmitry Shmidt <dimitrysh@google.com> wrote:
>>> On Fri, Jun 13, 2014 at 4:10 PM, Dmitry Shmidt <dimitrysh@google.com> wrote:
>>>> On Wed, Jun 11, 2014 at 2:30 PM, Jouni Malinen <j@w1.fi> wrote:
>>>>> On Mon, Jun 09, 2014 at 01:26:42AM -0700, Jithu Jance wrote:
>>>>>> Patch for Ignoring the RTM event for the dedicated P2P discovery
>>>>>> Interface. Without the patch the IFFUP operation on the interface
>>>>>> was failing (since there is no network device associated with the
>>>>>> interface).
>>>>>
>>>>>> +     if (drv->nlmode == NL80211_IFTYPE_P2P_DEVICE) {
>>>>>> +             wpa_printf(MSG_INFO, "nl80211: Ignore RTM_NEWLINK for P2P Discovery Interface");
>>>>>> +             return;
>>>>>> +     }
>>>>>
>>>>> Hmm.. How does NL82011_IFTYPE_P2P_DEVICE even get a RTM_NEWLINK event?
>>>>> The main point of that was to not have a netdev.. Could you please share
>>>>> a debug log showing what this looks like without this change?
>>>>
>>>> I think RTM_NEWLINK is received for group interface but this code fails it:
>>>>
>>>> if (!drv->if_disabled && !(ifi->ifi_flags & IFF_UP)) {
>>>>                 if (if_indextoname(ifi->ifi_index, namebuf) &&
>>>>                     linux_iface_up(drv->global->ioctl_sock,
>>>>                                    drv->first_bss->ifname) > 0) {  <<<< Here !!!
>>>>                         wpa_printf(MSG_DEBUG, "nl80211: Ignore interface down "
>>>>                                    "event since interface %s is up", namebuf);
>>>>                         return;
>>>>                 }
>>>>                 wpa_printf(MSG_DEBUG, "nl80211: Interface down");
>>>>
>>>>
>>>> Because namebuf == p2p-wlan0-0 and drv->first_bss->ifname == p2p-dev-wlan0,
>>>> where p2p-wlan0-0 is group interface, and p2p-dev-wlan0 is Non-NETDEV p2p.
>>>
>>> It looks like this change
>>>   http://hostap.epitest.fi/cgit/hostap/commit/?id=b36935be1a14341771b0fd5491808c3f6fdcb603
>>> causes nl80211_find_drv() to return wrong pointer.
>>>
>>>>
>>>>>
>>>>> --
>>>>> Jouni Malinen                                            PGP id EFC895FA
>>>>> _______________________________________________
>>>>> HostAP mailing list
>>>>> HostAP@lists.shmoo.com
>>>>> http://lists.shmoo.com/mailman/listinfo/hostap
>>> _______________________________________________
>>> HostAP mailing list
>>> HostAP@lists.shmoo.com
>>> http://lists.shmoo.com/mailman/listinfo/hostap

Comments

Jouni Malinen June 16, 2014, 10:41 p.m. UTC | #1
On Mon, Jun 16, 2014 at 07:56:12PM +0530, Jithu Jance wrote:
> Looks like last email got the patch as white space corrupted. So
> sending it again.

Thanks. Though, this one does not look much better in formatting. I
applied this manually and with a more accurate commit message.

The comments related to P2P Device interface (non-netdev) were a bit
confusing here. That is not really the root cause of the issue. The
commit you used as the model for this should really have included the
changes for this recently added add_ifidx() call as well. The main issue
here is in the parent interface getting additional ifindex values
registered for it even though that is not really supposed to happen with
the wpa_supplicant dynamic interface use cases. In the specific case of
the P2P Device interface, that resulted in the RTM_NEWLINK event of the
P2P group interface (which was added with that P2P Device interface as
the parent) to be processed with incorrect interface and then as a side
effect, of that, the P2P Device interface getting marked as unavailable
since it did not match the interface name.
diff mbox

Patch

diff --git a/src/drivers/driver_nl80211.c
b/src/drivers/driver_nl80211.c
index 7568653..baef09d 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -10177,8 +10177,16 @@  static int wpa_driver_nl80211_if_add(void
*priv, enum wpa_driver_if_type type,
    if (drv->global)
        drv->global->if_add_ifindex = ifidx;

-   if (ifidx > 0)
+   /*
+    * Some virtual interfaces need to process EAPOL packets and
events on
+    * the parent interface. This is used mainly with hostapd.
+    */
+   if ((ifidx > 0) && (drv->hostapd ||
+       nlmode == NL80211_IFTYPE_AP_VLAN ||
+       nlmode == NL80211_IFTYPE_WDS ||
+       nlmode == NL80211_IFTYPE_MONITOR)) {
        add_ifidx(drv, ifidx);
+   }