Re: igmp: set rtableid on new mbufs
On Wed, Dec 14, 2016 at 06:59:42PM +0100, Martin Pieuchot wrote:
> On 14/12/16(Wed) 16:54, Rafael Zalamena wrote:
> > After running the igmpproxy in multiple domains I noticed that the kernel
> > started complaining about sending packets on wrong domains. Here is the
> > exact message:
> > "
> > vio1: trying to send packet on wrong domain. if 1 vs. mbuf 0
> > "
> >
> > After some debugging I traced the problem to the igmp_sendpkt() function
> > and it seems that it is missing to set the mbuf rdomain, so this is
> > exactly what this diff does.
>
> It doesn't make sense to call if_get(9) when all the callers of
> igmp_sendpkt() already have a reference to the sending ifp. if_get(9)
> has a cost and adds complexity. I'd rather pass ifp or the rdomain to
> igmp_sendpkt().
Following mpi@'s suggestion here is a new diff that removes the
if_get()/if_put() from igmp_sendpkt() and make it the callers
responsability.
ok?
Index: sys/netinet/igmp.c
===
RCS file: /home/obsdcvs/src/sys/netinet/igmp.c,v
retrieving revision 1.57
diff -u -p -r1.57 igmp.c
--- sys/netinet/igmp.c 14 Dec 2016 17:15:56 - 1.57
+++ sys/netinet/igmp.c 16 Dec 2016 11:19:42 -
@@ -104,7 +104,7 @@ static struct mbuf *router_alert;
struct igmpstat igmpstat;
void igmp_checktimer(struct ifnet *);
-void igmp_sendpkt(struct in_multi *, int, in_addr_t);
+void igmp_sendpkt(struct ifnet *, struct in_multi *, int, in_addr_t);
int rti_fill(struct in_multi *);
struct router_info * rti_find(struct ifnet *);
void igmp_input_if(struct ifnet *, struct mbuf *, int);
@@ -509,7 +509,7 @@ igmp_joingroup(struct in_multi *inm)
if ((i = rti_fill(inm)) == -1)
goto out;
- igmp_sendpkt(inm, i, 0);
+ igmp_sendpkt(ifp, inm, i, 0);
inm->inm_state = IGMP_DELAYING_MEMBER;
inm->inm_timer = IGMP_RANDOM_DELAY(
IGMP_MAX_HOST_REPORT_DELAY * PR_FASTHZ);
@@ -534,7 +534,8 @@ igmp_leavegroup(struct in_multi *inm)
if (!IN_LOCAL_GROUP(inm->inm_addr.s_addr) &&
ifp && (ifp->if_flags & IFF_LOOPBACK) == 0)
if (inm->inm_rti->rti_type != IGMP_v1_ROUTER)
- igmp_sendpkt(inm, IGMP_HOST_LEAVE_MESSAGE,
+ igmp_sendpkt(ifp, inm,
+ IGMP_HOST_LEAVE_MESSAGE,
INADDR_ALLROUTERS_GROUP);
break;
case IGMP_LAZY_MEMBER:
@@ -582,10 +583,10 @@ igmp_checktimer(struct ifnet *ifp)
} else if (--inm->inm_timer == 0) {
if (inm->inm_state == IGMP_DELAYING_MEMBER) {
if (inm->inm_rti->rti_type == IGMP_v1_ROUTER)
- igmp_sendpkt(inm,
+ igmp_sendpkt(ifp, inm,
IGMP_v1_HOST_MEMBERSHIP_REPORT, 0);
else
- igmp_sendpkt(inm,
+ igmp_sendpkt(ifp, inm,
IGMP_v2_HOST_MEMBERSHIP_REPORT, 0);
inm->inm_state = IGMP_IDLE_MEMBER;
}
@@ -611,22 +612,17 @@ igmp_slowtimo(void)
}
void
-igmp_sendpkt(struct in_multi *inm, int type, in_addr_t addr)
+igmp_sendpkt(struct ifnet *ifp, struct in_multi *inm, int type,
+in_addr_t addr)
{
- struct ifnet *ifp;
struct mbuf *m;
struct igmp *igmp;
struct ip *ip;
struct ip_moptions imo;
- if ((ifp = if_get(inm->inm_ifidx)) == NULL)
- return;
-
MGETHDR(m, M_DONTWAIT, MT_HEADER);
- if (m == NULL) {
- if_put(ifp);
+ if (m == NULL)
return;
- }
/*
* Assume max_linkhdr + sizeof(struct ip) + IGMP_MINLEN
@@ -674,7 +670,6 @@ igmp_sendpkt(struct in_multi *inm, int t
#endif /* MROUTING */
ip_output(m, router_alert, NULL, IP_MULTICASTOPTS, &imo, NULL, 0);
- if_put(ifp);
++igmpstat.igps_snd_reports;
}
Re: igmp: set rtableid on new mbufs
On 14/12/16(Wed) 16:54, Rafael Zalamena wrote:
> After running the igmpproxy in multiple domains I noticed that the kernel
> started complaining about sending packets on wrong domains. Here is the
> exact message:
> "
> vio1: trying to send packet on wrong domain. if 1 vs. mbuf 0
> "
>
> After some debugging I traced the problem to the igmp_sendpkt() function
> and it seems that it is missing to set the mbuf rdomain, so this is
> exactly what this diff does.
It doesn't make sense to call if_get(9) when all the callers of
igmp_sendpkt() already have a reference to the sending ifp. if_get(9)
has a cost and adds complexity. I'd rather pass ifp or the rdomain to
igmp_sendpkt().
> Index: sys/netinet/igmp.c
> ===
> RCS file: /home/obsdcvs/src/sys/netinet/igmp.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 igmp.c
> --- sys/netinet/igmp.c5 Dec 2016 15:31:43 - 1.56
> +++ sys/netinet/igmp.c14 Dec 2016 15:40:08 -
> @@ -613,14 +613,21 @@ igmp_slowtimo(void)
> void
> igmp_sendpkt(struct in_multi *inm, int type, in_addr_t addr)
> {
> + struct ifnet *ifp;
> struct mbuf *m;
> struct igmp *igmp;
> struct ip *ip;
> struct ip_moptions imo;
>
> + if ((ifp = if_get(inm->inm_ifidx)) == NULL)
> + return;
> +
> MGETHDR(m, M_DONTWAIT, MT_HEADER);
> - if (m == NULL)
> + if (m == NULL) {
> + if_put(ifp);
> return;
> + }
> +
> /*
>* Assume max_linkhdr + sizeof(struct ip) + IGMP_MINLEN
>* is smaller than mbuf size returned by MGETHDR.
> @@ -652,6 +659,7 @@ igmp_sendpkt(struct in_multi *inm, int t
> m->m_data -= sizeof(struct ip);
> m->m_len += sizeof(struct ip);
>
> + m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
> imo.imo_ifidx = inm->inm_ifidx;
> imo.imo_ttl = 1;
>
> @@ -666,6 +674,7 @@ igmp_sendpkt(struct in_multi *inm, int t
> #endif /* MROUTING */
>
> ip_output(m, router_alert, NULL, IP_MULTICASTOPTS, &imo, NULL, 0);
> + if_put(ifp);
>
> ++igmpstat.igps_snd_reports;
> }
>
igmp: set rtableid on new mbufs
After running the igmpproxy in multiple domains I noticed that the kernel
started complaining about sending packets on wrong domains. Here is the
exact message:
"
vio1: trying to send packet on wrong domain. if 1 vs. mbuf 0
"
After some debugging I traced the problem to the igmp_sendpkt() function
and it seems that it is missing to set the mbuf rdomain, so this is
exactly what this diff does.
ok?
Index: sys/netinet/igmp.c
===
RCS file: /home/obsdcvs/src/sys/netinet/igmp.c,v
retrieving revision 1.56
diff -u -p -r1.56 igmp.c
--- sys/netinet/igmp.c 5 Dec 2016 15:31:43 - 1.56
+++ sys/netinet/igmp.c 14 Dec 2016 15:40:08 -
@@ -613,14 +613,21 @@ igmp_slowtimo(void)
void
igmp_sendpkt(struct in_multi *inm, int type, in_addr_t addr)
{
+ struct ifnet *ifp;
struct mbuf *m;
struct igmp *igmp;
struct ip *ip;
struct ip_moptions imo;
+ if ((ifp = if_get(inm->inm_ifidx)) == NULL)
+ return;
+
MGETHDR(m, M_DONTWAIT, MT_HEADER);
- if (m == NULL)
+ if (m == NULL) {
+ if_put(ifp);
return;
+ }
+
/*
* Assume max_linkhdr + sizeof(struct ip) + IGMP_MINLEN
* is smaller than mbuf size returned by MGETHDR.
@@ -652,6 +659,7 @@ igmp_sendpkt(struct in_multi *inm, int t
m->m_data -= sizeof(struct ip);
m->m_len += sizeof(struct ip);
+ m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
imo.imo_ifidx = inm->inm_ifidx;
imo.imo_ttl = 1;
@@ -666,6 +674,7 @@ igmp_sendpkt(struct in_multi *inm, int t
#endif /* MROUTING */
ip_output(m, router_alert, NULL, IP_MULTICASTOPTS, &imo, NULL, 0);
+ if_put(ifp);
++igmpstat.igps_snd_reports;
}
