Stop G/C mbufs in if_detach()

2015-06-23 Thread Martin Pieuchot
When an interface is detached or destroyed the CPU executing if_detach()
removes all the mbufs received by this interface on three queues:
ARP, IPv4 and IPv6 protocol queues.

This made sense to avoid referencing a dangling "rcvif" pointer. But now
mbufs contain unique interface indexes and protocol interrupt routines
handle just fine the case where if_get() returns a NULL pointer.

So I'd like to get rid of this explicit garbage collection.  Note that
this will leave mbufs on the protocol queues until the next netisr is
executed for the corresponding queue.  This is a functional change but
I don't think it's a problem.  It only matters if you destroy your only
pseudo interface or unplug your single USB interface without replugging
it.

Comments, oks?

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.341
diff -u -p -r1.341 if.c
--- net/if.c23 Jun 2015 09:42:23 -  1.341
+++ net/if.c23 Jun 2015 10:54:04 -
@@ -128,8 +128,6 @@ voidif_attachsetup(struct ifnet *);
 void   if_attachdomain1(struct ifnet *);
 void   if_attach_common(struct ifnet *);
 
-intif_detach_filter(void *, const struct mbuf *);
-void   if_detach_queues(struct ifnet *, struct niqueue *);
 void   if_detached_start(struct ifnet *);
 intif_detached_ioctl(struct ifnet *, u_long, caddr_t);
 
@@ -644,23 +642,6 @@ if_detach(struct ifnet *ifp)
pfi_detach_ifnet(ifp);
 #endif
 
-   /*
-* remove packets came from ifp, from software interrupt queues.
-* net/netisr_dispatch.h is not usable, as some of them use
-* strange queue names.
-*/
-#define IF_DETACH_QUEUES(x) \
-do { \
-   extern struct niqueue x; \
-   if_detach_queues(ifp, & x); \
-} while (0)
-   IF_DETACH_QUEUES(arpintrq);
-   IF_DETACH_QUEUES(ipintrq);
-#ifdef INET6
-   IF_DETACH_QUEUES(ip6intrq);
-#endif
-#undef IF_DETACH_QUEUES
-
/* Remove the interface from the list of all interfaces.  */
TAILQ_REMOVE(&ifnet, ifp, if_list);
if (ISSET(ifp->if_xflags, IFXF_TXREADY))
@@ -701,34 +682,6 @@ do { \
 
ifindex2ifnet[ifp->if_index] = NULL;
splx(s);
-}
-
-int
-if_detach_filter(void *ctx, const struct mbuf *m)
-{
-   struct ifnet *ifp = ctx;
-
-#ifdef DIAGNOSTIC
-   if ((m->m_flags & M_PKTHDR) == 0)
-   return (0);
-#endif
-
-   return (m->m_pkthdr.ph_ifidx == ifp->if_index);
-}
-
-void
-if_detach_queues(struct ifnet *ifp, struct niqueue *niq)
-{
-   struct mbuf *m0, *m;
-
-   m0 = niq_filter(niq, if_detach_filter, ifp);
-   while (m0 != NULL) {
-   m = m0;
-   m0 = m->m_nextpkt;
-
-   m->m_nextpkt = NULL;
-   m_freem(m);
-   }
 }
 
 /*



Re: Stop G/C mbufs in if_detach()

2015-06-23 Thread Claudio Jeker
On Tue, Jun 23, 2015 at 03:01:54PM +0200, Martin Pieuchot wrote:
> When an interface is detached or destroyed the CPU executing if_detach()
> removes all the mbufs received by this interface on three queues:
> ARP, IPv4 and IPv6 protocol queues.
> 
> This made sense to avoid referencing a dangling "rcvif" pointer. But now
> mbufs contain unique interface indexes and protocol interrupt routines
> handle just fine the case where if_get() returns a NULL pointer.
> 
> So I'd like to get rid of this explicit garbage collection.  Note that
> this will leave mbufs on the protocol queues until the next netisr is
> executed for the corresponding queue.  This is a functional change but
> I don't think it's a problem.  It only matters if you destroy your only
> pseudo interface or unplug your single USB interface without replugging
> it.
> 
> Comments, oks?

OK claudio@ (there are many more queues that would need to be cleaned if
we wanted this)
 
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.341
> diff -u -p -r1.341 if.c
> --- net/if.c  23 Jun 2015 09:42:23 -  1.341
> +++ net/if.c  23 Jun 2015 10:54:04 -
> @@ -128,8 +128,6 @@ void  if_attachsetup(struct ifnet *);
>  void if_attachdomain1(struct ifnet *);
>  void if_attach_common(struct ifnet *);
>  
> -int  if_detach_filter(void *, const struct mbuf *);
> -void if_detach_queues(struct ifnet *, struct niqueue *);
>  void if_detached_start(struct ifnet *);
>  int  if_detached_ioctl(struct ifnet *, u_long, caddr_t);
>  
> @@ -644,23 +642,6 @@ if_detach(struct ifnet *ifp)
>   pfi_detach_ifnet(ifp);
>  #endif
>  
> - /*
> -  * remove packets came from ifp, from software interrupt queues.
> -  * net/netisr_dispatch.h is not usable, as some of them use
> -  * strange queue names.
> -  */
> -#define IF_DETACH_QUEUES(x) \
> -do { \
> - extern struct niqueue x; \
> - if_detach_queues(ifp, & x); \
> -} while (0)
> - IF_DETACH_QUEUES(arpintrq);
> - IF_DETACH_QUEUES(ipintrq);
> -#ifdef INET6
> - IF_DETACH_QUEUES(ip6intrq);
> -#endif
> -#undef IF_DETACH_QUEUES
> -
>   /* Remove the interface from the list of all interfaces.  */
>   TAILQ_REMOVE(&ifnet, ifp, if_list);
>   if (ISSET(ifp->if_xflags, IFXF_TXREADY))
> @@ -701,34 +682,6 @@ do { \
>  
>   ifindex2ifnet[ifp->if_index] = NULL;
>   splx(s);
> -}
> -
> -int
> -if_detach_filter(void *ctx, const struct mbuf *m)
> -{
> - struct ifnet *ifp = ctx;
> -
> -#ifdef DIAGNOSTIC
> - if ((m->m_flags & M_PKTHDR) == 0)
> - return (0);
> -#endif
> -
> - return (m->m_pkthdr.ph_ifidx == ifp->if_index);
> -}
> -
> -void
> -if_detach_queues(struct ifnet *ifp, struct niqueue *niq)
> -{
> - struct mbuf *m0, *m;
> -
> - m0 = niq_filter(niq, if_detach_filter, ifp);
> - while (m0 != NULL) {
> - m = m0;
> - m0 = m->m_nextpkt;
> -
> - m->m_nextpkt = NULL;
> - m_freem(m);
> - }
>  }
>  
>  /*
> 

-- 
:wq Claudio