On Fri, Oct 18, 2013 at 01:00:25PM +0200, Martin Pieuchot wrote:
> On 18/10/13(Fri) 12:45, Alexander Bluhm wrote:
> >
> > Ethernet drivers connected via USB might sleep when their multicast
> > group filter is modified. Unfortunately this happens from softclock
> > or softnet interrupt when IPv6 decides to unconfigure its addresses
> > automatically.
> >
> > An obvious solution is to use a work queue. I have put the workq
> > storage into struct in6_multi_mship. That requires to include
> > sys/workq.h before compiling this struct.
>
> One problem with a work queue is that you cannot guarantee the interface
> will still be here when the task will be scheduled, so passing a pointer
> might not be a good idea.
>
> That is why I wanted to change the if_get() API to use unique index for
> each interface...
>
> But maybe there's another solution at this problem than using a workq...
Now I use the if_index to detect that the interface is gone.
I this a better approach?
bluhm
Index: netinet6/in6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.122
diff -u -p -u -p -r1.122 in6.c
--- netinet6/in6.c 24 Oct 2013 11:20:18 -0000 1.122
+++ netinet6/in6.c 30 Oct 2013 15:01:20 -0000
@@ -124,6 +124,7 @@ int in6_lifaddr_ioctl(struct socket *, u
int in6_ifinit(struct ifnet *, struct in6_ifaddr *, int);
void in6_unlink_ifa(struct in6_ifaddr *, struct ifnet *);
void in6_ifloop_request(int, struct ifaddr *);
+void in6_leavegroup_task(void *, void *);
const struct sockaddr_in6 sa6_any = {
sizeof(sa6_any), AF_INET6, 0, 0, IN6ADDR_ANY_INIT, 0
@@ -1777,16 +1778,18 @@ in6_delmulti(struct in6_multi *in6m)
ifafree(&in6m->in6m_ia->ia_ifa); /* release reference */
}
- /*
- * Notify the network driver to update its multicast
- * reception filter.
- */
- bzero(&ifr.ifr_addr, sizeof(struct sockaddr_in6));
- ifr.ifr_addr.sin6_len = sizeof(struct sockaddr_in6);
- ifr.ifr_addr.sin6_family = AF_INET6;
- ifr.ifr_addr.sin6_addr = in6m->in6m_addr;
- (*in6m->in6m_ifp->if_ioctl)(in6m->in6m_ifp,
- SIOCDELMULTI, (caddr_t)&ifr);
+ if (in6m->in6m_ifp != NULL) {
+ /*
+ * Notify the network driver to update its multicast
+ * reception filter.
+ */
+ bzero(&ifr.ifr_addr, sizeof(struct sockaddr_in6));
+ ifr.ifr_addr.sin6_len = sizeof(struct sockaddr_in6);
+ ifr.ifr_addr.sin6_family = AF_INET6;
+ ifr.ifr_addr.sin6_addr = in6m->in6m_addr;
+ (*in6m->in6m_ifp->if_ioctl)(in6m->in6m_ifp,
+ SIOCDELMULTI, (caddr_t)&ifr);
+ }
free(in6m, M_IPMADDR);
}
splx(s);
@@ -1814,11 +1817,26 @@ in6_joingroup(struct ifnet *ifp, struct
int
in6_leavegroup(struct in6_multi_mship *imm)
{
-
if (imm->i6mm_maddr)
- in6_delmulti(imm->i6mm_maddr);
- free(imm, M_IPMADDR);
+ workq_queue_task(NULL, &imm->wqt, 0, in6_leavegroup_task, imm,
+ (void *)(unsigned long)imm->i6mm_maddr->in6m_ifp->if_index);
+ else
+ free(imm, M_IPMADDR);
return 0;
+}
+
+void
+in6_leavegroup_task(void *arg1, void *arg2)
+{
+ struct in6_multi_mship *imm = arg1;
+ unsigned int index = (unsigned long)arg2;
+
+ /* If interface has been be freed, avoid call to if_ioctl(). */
+ if (if_get(index) != imm->i6mm_maddr->in6m_ifp)
+ imm->i6mm_maddr->in6m_ifp = NULL;
+
+ in6_delmulti(imm->i6mm_maddr);
+ free(imm, M_IPMADDR);
}
/*
Index: netinet6/in6_var.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_var.h,v
retrieving revision 1.44
diff -u -p -u -p -r1.44 in6_var.h
--- netinet6/in6_var.h 24 Oct 2013 11:31:43 -0000 1.44
+++ netinet6/in6_var.h 30 Oct 2013 15:01:20 -0000
@@ -64,6 +64,8 @@
#ifndef _NETINET6_IN6_VAR_H_
#define _NETINET6_IN6_VAR_H_
+#include <sys/workq.h>
+
/*
* Interface address, Internet version. One of these structures
* is allocated for each interface with an Internet address.
@@ -480,6 +482,7 @@ do {
\
* belongs to.
*/
struct in6_multi_mship {
+ struct workq_task wqt; /* Allow network driver to sleep */
struct in6_multi *i6mm_maddr; /* Multicast address pointer */
LIST_ENTRY(in6_multi_mship) i6mm_chain; /* multicast options chain */
};
