Re: svn commit: r354149 - head/sys/net

2019-11-11 Thread Renato Botelho
On 09/11/19 01:16, Gleb Smirnoff wrote:
>   Renato,
> 
> can you please try out the attached patch?

I upgraded to r354607 with the patch applied and the problem is now fixed.

Thanks
-- 
Renato Botelho
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r354149 - head/sys/net

2019-11-08 Thread Gleb Smirnoff
  Renato,

can you please try out the attached patch?

On Fri, Nov 08, 2019 at 01:27:30PM -0300, Renato Botelho wrote:
R> On 29/10/19 14:36, Gleb Smirnoff wrote:
R> > Author: glebius
R> > Date: Tue Oct 29 17:36:06 2019
R> > New Revision: 354149
R> > URL: https://svnweb.freebsd.org/changeset/base/354149
R> > 
R> > Log:
R> >   There is a long standing problem with multicast programming for NICs
R> >   and IPv6.  With IPv6 we may call if_addmulti() in context of processing
R> >   of an incoming packet.  Usually this is interrupt context.  While most
R> >   of the NIC drivers are able to reprogram multicast filters without
R> >   sleeping, some of them can't.  An example is e1000 family of drivers.
R> >   With iflib conversion the problem was somewhat hidden.  Iflib processes
R> >   packets in private taskqueue, so going to sleep doesn't trigger an
R> >   assertion.  However, the sleep would block operation of the driver and
R> >   following incoming packets would fill the ring and eventually would
R> >   start being dropped.  Enabling epoch for the full time of a packet
R> >   processing again started to trigger assertions for e1000.
R> >   
R> >   Fix this problem once and for all using a general taskqueue to call
R> >   if_ioctl() method in all cases when if_addmulti() is called in a
R> >   non sleeping context.  Note that nobody cares about returned value.
R> >   
R> >   Reviewed by: hselasky, kib
R> >   Differential Revision: https://reviews.freebsd.org/D22154
R> 
R> Hi Gleb,
R> 
R> I upgraded my laptop running 13-CURRENT from r354133 to r354437 and it
R> crashed during boot as you can see in the pictures [1].  It seems like
R> it crashed while it was configuring network.
R> 
R> After bisect I managed to boot fine with r354148 and reproduce the crash
R> with this revision applied.
R> 
R> Here is the relevant portion of my /etc/rc.conf:
R> 
R> # Lagg
R> ifconfig_em0="up"
R> wlans_iwn0="wlan0"
R> ifconfig_wlan0="WPA"
R> create_args_wlan0="wlanaddr 3c:97:0e:48:3f:f8 up"
R> cloned_interfaces="lagg0"
R> ifconfig_lagg0="up laggproto failover laggport em0 laggport wlan0 DHCP"
R> ifconfig_lagg0_ipv6="inet6 accept_rtadv"
R> rtsold_enable="YES"
R> 
R> [1] https://imgur.com/a/lBtq3FW
R> -- 
R> Renato Botelho

-- 
Gleb Smirnoff
Index: sys/net/if.c
===
--- sys/net/if.c	(revision 354565)
+++ sys/net/if.c	(working copy)
@@ -3571,7 +3571,9 @@ if_siocaddmulti(void *arg, int pending)
 	if (pending > 1)
 		if_printf(ifp, "%d SIOCADDMULTI coalesced\n", pending);
 #endif
+	CURVNET_SET(ifp->if_vnet);
 	(void )(*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0);
+	CURVNET_RESTORE();
 }
 
 /*
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r354149 - head/sys/net

2019-11-08 Thread Renato Botelho
On 08/11/19 13:27, Renato Botelho wrote:
> On 29/10/19 14:36, Gleb Smirnoff wrote:
>> Author: glebius
>> Date: Tue Oct 29 17:36:06 2019
>> New Revision: 354149
>> URL: https://svnweb.freebsd.org/changeset/base/354149
>>
>> Log:
>>   There is a long standing problem with multicast programming for NICs
>>   and IPv6.  With IPv6 we may call if_addmulti() in context of processing
>>   of an incoming packet.  Usually this is interrupt context.  While most
>>   of the NIC drivers are able to reprogram multicast filters without
>>   sleeping, some of them can't.  An example is e1000 family of drivers.
>>   With iflib conversion the problem was somewhat hidden.  Iflib processes
>>   packets in private taskqueue, so going to sleep doesn't trigger an
>>   assertion.  However, the sleep would block operation of the driver and
>>   following incoming packets would fill the ring and eventually would
>>   start being dropped.  Enabling epoch for the full time of a packet
>>   processing again started to trigger assertions for e1000.
>>   
>>   Fix this problem once and for all using a general taskqueue to call
>>   if_ioctl() method in all cases when if_addmulti() is called in a
>>   non sleeping context.  Note that nobody cares about returned value.
>>   
>>   Reviewed by:   hselasky, kib
>>   Differential Revision:   https://reviews.freebsd.org/D22154
> 
> Hi Gleb,
> 
> I upgraded my laptop running 13-CURRENT from r354133 to r354437 and it
> crashed during boot as you can see in the pictures [1].  It seems like
> it crashed while it was configuring network.
> 
> After bisect I managed to boot fine with r354148 and reproduce the crash
> with this revision applied.
> 
> Here is the relevant portion of my /etc/rc.conf:
> 
> # Lagg
> ifconfig_em0="up"
> wlans_iwn0="wlan0"
> ifconfig_wlan0="WPA"
> create_args_wlan0="wlanaddr 3c:97:0e:48:3f:f8 up"
> cloned_interfaces="lagg0"
> ifconfig_lagg0="up laggproto failover laggport em0 laggport wlan0 DHCP"
> ifconfig_lagg0_ipv6="inet6 accept_rtadv"
> rtsold_enable="YES"
> 
> [1] https://imgur.com/a/lBtq3FW

I forgot to mention.  If I disable IPv6 on lagg interface the crash is gone.

-- 
Renato Botelho
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r354149 - head/sys/net

2019-11-08 Thread Renato Botelho
On 29/10/19 14:36, Gleb Smirnoff wrote:
> Author: glebius
> Date: Tue Oct 29 17:36:06 2019
> New Revision: 354149
> URL: https://svnweb.freebsd.org/changeset/base/354149
> 
> Log:
>   There is a long standing problem with multicast programming for NICs
>   and IPv6.  With IPv6 we may call if_addmulti() in context of processing
>   of an incoming packet.  Usually this is interrupt context.  While most
>   of the NIC drivers are able to reprogram multicast filters without
>   sleeping, some of them can't.  An example is e1000 family of drivers.
>   With iflib conversion the problem was somewhat hidden.  Iflib processes
>   packets in private taskqueue, so going to sleep doesn't trigger an
>   assertion.  However, the sleep would block operation of the driver and
>   following incoming packets would fill the ring and eventually would
>   start being dropped.  Enabling epoch for the full time of a packet
>   processing again started to trigger assertions for e1000.
>   
>   Fix this problem once and for all using a general taskqueue to call
>   if_ioctl() method in all cases when if_addmulti() is called in a
>   non sleeping context.  Note that nobody cares about returned value.
>   
>   Reviewed by:hselasky, kib
>   Differential Revision:https://reviews.freebsd.org/D22154

Hi Gleb,

I upgraded my laptop running 13-CURRENT from r354133 to r354437 and it
crashed during boot as you can see in the pictures [1].  It seems like
it crashed while it was configuring network.

After bisect I managed to boot fine with r354148 and reproduce the crash
with this revision applied.

Here is the relevant portion of my /etc/rc.conf:

# Lagg
ifconfig_em0="up"
wlans_iwn0="wlan0"
ifconfig_wlan0="WPA"
create_args_wlan0="wlanaddr 3c:97:0e:48:3f:f8 up"
cloned_interfaces="lagg0"
ifconfig_lagg0="up laggproto failover laggport em0 laggport wlan0 DHCP"
ifconfig_lagg0_ipv6="inet6 accept_rtadv"
rtsold_enable="YES"

[1] https://imgur.com/a/lBtq3FW
-- 
Renato Botelho
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


svn commit: r354149 - head/sys/net

2019-10-29 Thread Gleb Smirnoff
Author: glebius
Date: Tue Oct 29 17:36:06 2019
New Revision: 354149
URL: https://svnweb.freebsd.org/changeset/base/354149

Log:
  There is a long standing problem with multicast programming for NICs
  and IPv6.  With IPv6 we may call if_addmulti() in context of processing
  of an incoming packet.  Usually this is interrupt context.  While most
  of the NIC drivers are able to reprogram multicast filters without
  sleeping, some of them can't.  An example is e1000 family of drivers.
  With iflib conversion the problem was somewhat hidden.  Iflib processes
  packets in private taskqueue, so going to sleep doesn't trigger an
  assertion.  However, the sleep would block operation of the driver and
  following incoming packets would fill the ring and eventually would
  start being dropped.  Enabling epoch for the full time of a packet
  processing again started to trigger assertions for e1000.
  
  Fix this problem once and for all using a general taskqueue to call
  if_ioctl() method in all cases when if_addmulti() is called in a
  non sleeping context.  Note that nobody cares about returned value.
  
  Reviewed by:  hselasky, kib
  Differential Revision:  https://reviews.freebsd.org/D22154

Modified:
  head/sys/net/if.c
  head/sys/net/if_var.h

Modified: head/sys/net/if.c
==
--- head/sys/net/if.c   Tue Oct 29 17:28:25 2019(r354148)
+++ head/sys/net/if.c   Tue Oct 29 17:36:06 2019(r354149)
@@ -271,6 +271,7 @@ static int  if_getgroupmembers(struct ifgroupreq *);
 static voidif_delgroups(struct ifnet *);
 static voidif_attach_internal(struct ifnet *, int, struct if_clone *);
 static int if_detach_internal(struct ifnet *, int, struct if_clone **);
+static voidif_siocaddmulti(void *, int);
 #ifdef VIMAGE
 static voidif_vmove(struct ifnet *, struct vnet *);
 #endif
@@ -556,6 +557,7 @@ if_alloc_domain(u_char type, int numa_domain)
 
IF_ADDR_LOCK_INIT(ifp);
TASK_INIT(&ifp->if_linktask, 0, do_link_state_change, ifp);
+   TASK_INIT(&ifp->if_addmultitask, 0, if_siocaddmulti, ifp);
ifp->if_afdata_initialized = 0;
IF_AFDATA_LOCK_INIT(ifp);
CK_STAILQ_INIT(&ifp->if_addrhead);
@@ -1131,6 +1133,7 @@ if_detach_internal(struct ifnet *ifp, int vmove, struc
if_delgroups(ifp);
 
taskqueue_drain(taskqueue_swi, &ifp->if_linktask);
+   taskqueue_drain(taskqueue_swi, &ifp->if_addmultitask);
 
/*
 * Check if this is a cloned interface or not. Must do even if
@@ -3538,7 +3541,10 @@ if_addmulti(struct ifnet *ifp, struct sockaddr *sa,
 * interface to let them know about it.
 */
if (ifp->if_ioctl != NULL) {
-   (void) (*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0);
+   if (THREAD_CAN_SLEEP())
+   (void )(*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0);
+   else
+   taskqueue_enqueue(taskqueue_swi, &ifp->if_addmultitask);
}
 
if ((llsa != NULL) && (llsa != (struct sockaddr *)&sdl))
@@ -3553,6 +3559,19 @@ free_llsa_out:
 unlock_out:
IF_ADDR_WUNLOCK(ifp);
return (error);
+}
+
+static void
+if_siocaddmulti(void *arg, int pending)
+{
+   struct ifnet *ifp;
+
+   ifp = arg;
+#ifdef DIAGNOSTIC
+   if (pending > 1)
+   if_printf(ifp, "%d SIOCADDMULTI coalesced\n", pending);
+#endif
+   (void )(*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0);
 }
 
 /*

Modified: head/sys/net/if_var.h
==
--- head/sys/net/if_var.h   Tue Oct 29 17:28:25 2019(r354148)
+++ head/sys/net/if_var.h   Tue Oct 29 17:36:06 2019(r354149)
@@ -317,6 +317,7 @@ struct ifnet {
 
struct  ifaltq if_snd;  /* output queue (includes altq) */
struct  task if_linktask;   /* task for link change events */
+   struct  task if_addmultitask;   /* task for SIOCADDMULTI */
 
/* Addresses of different protocol families assigned to this if. */
struct mtx if_addr_lock;/* lock to protect address lists */
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"