Re: Do not reroll IFP_TO_IA
On Fri, Sep 06, 2013 at 11:02:14AM +0200, Martin Pieuchot wrote: > On 05/09/13(Thu) 19:28, Alexander Bluhm wrote: > > On Thu, Sep 05, 2013 at 10:54:53AM +0200, Martin Pieuchot wrote: > > > Diff below makes use of IFP_TO_IA() instead of rolling our own copy. > > > > > > For now there's no functional change, but I'd like to get this in so > > > that once our multicast code can stop relying on global lists, we only > > > need to modify the macro. > > > > > > ok? > > > > The old code did run into the EADDRNOTAVAIL case, if the routing > > domain did not match. Now you don't return. > > > > > > I think you need someting like this; > > > > ifp = mopts->imo_multicast_ifp; > > if (ifp != NULL) { > > IFP_TO_IA(ifp, ia); > > if (ia == NULL || > > ifp->if_rdomain != rtable_l2(rtableid)) { > > *errorp = EADDRNOTAVAIL; > > return NULL; > > } > > You're right. That means we can have a non-NULL ia only if the routing > domain match. So I propose this: OK bluhm@ > > Index: in_pcb.c > === > RCS file: /home/ncvs/src/sys/netinet/in_pcb.c,v > retrieving revision 1.139 > diff -u -p -r1.139 in_pcb.c > --- in_pcb.c 1 Jun 2013 13:25:40 - 1.139 > +++ in_pcb.c 6 Sep 2013 09:00:05 - > @@ -794,13 +794,12 @@ in_selectsrc(struct sockaddr_in *sin, st > if (IN_MULTICAST(sin->sin_addr.s_addr) && mopts != NULL) { > struct ifnet *ifp; > > - if (mopts->imo_multicast_ifp != NULL) { > - ifp = mopts->imo_multicast_ifp; > - TAILQ_FOREACH(ia, &in_ifaddr, ia_list) > - if (ia->ia_ifp == ifp && > - rtable_l2(rtableid) == ifp->if_rdomain) > - break; > - if (ia == 0) { > + ifp = mopts->imo_multicast_ifp; > + if (ifp != NULL) { > + if (ifp->if_rdomain == rtable_l2(rtableid)) > + IFP_TO_IA(ifp, ia); > + > + if (ia == NULL) { > *errorp = EADDRNOTAVAIL; > return NULL; > }
Re: Do not reroll IFP_TO_IA
On 05/09/13(Thu) 19:28, Alexander Bluhm wrote: > On Thu, Sep 05, 2013 at 10:54:53AM +0200, Martin Pieuchot wrote: > > Diff below makes use of IFP_TO_IA() instead of rolling our own copy. > > > > For now there's no functional change, but I'd like to get this in so > > that once our multicast code can stop relying on global lists, we only > > need to modify the macro. > > > > ok? > > The old code did run into the EADDRNOTAVAIL case, if the routing > domain did not match. Now you don't return. > > > I think you need someting like this; > > ifp = mopts->imo_multicast_ifp; > if (ifp != NULL) { > IFP_TO_IA(ifp, ia); > if (ia == NULL || > ifp->if_rdomain != rtable_l2(rtableid)) { > *errorp = EADDRNOTAVAIL; > return NULL; > } You're right. That means we can have a non-NULL ia only if the routing domain match. So I propose this: Index: in_pcb.c === RCS file: /home/ncvs/src/sys/netinet/in_pcb.c,v retrieving revision 1.139 diff -u -p -r1.139 in_pcb.c --- in_pcb.c1 Jun 2013 13:25:40 - 1.139 +++ in_pcb.c6 Sep 2013 09:00:05 - @@ -794,13 +794,12 @@ in_selectsrc(struct sockaddr_in *sin, st if (IN_MULTICAST(sin->sin_addr.s_addr) && mopts != NULL) { struct ifnet *ifp; - if (mopts->imo_multicast_ifp != NULL) { - ifp = mopts->imo_multicast_ifp; - TAILQ_FOREACH(ia, &in_ifaddr, ia_list) - if (ia->ia_ifp == ifp && - rtable_l2(rtableid) == ifp->if_rdomain) - break; - if (ia == 0) { + ifp = mopts->imo_multicast_ifp; + if (ifp != NULL) { + if (ifp->if_rdomain == rtable_l2(rtableid)) + IFP_TO_IA(ifp, ia); + + if (ia == NULL) { *errorp = EADDRNOTAVAIL; return NULL; }
Re: Do not reroll IFP_TO_IA
On Thu, Sep 05, 2013 at 10:54:53AM +0200, Martin Pieuchot wrote: > Diff below makes use of IFP_TO_IA() instead of rolling our own copy. > > For now there's no functional change, but I'd like to get this in so > that once our multicast code can stop relying on global lists, we only > need to modify the macro. > > ok? The old code did run into the EADDRNOTAVAIL case, if the routing domain did not match. Now you don't return. I think you need someting like this; ifp = mopts->imo_multicast_ifp; if (ifp != NULL) { IFP_TO_IA(ifp, ia); if (ia == NULL || ifp->if_rdomain != rtable_l2(rtableid)) { *errorp = EADDRNOTAVAIL; return NULL; } bluhm > > Index: netinet/in_pcb.c > === > RCS file: /home/ncvs/src/sys/netinet/in_pcb.c,v > retrieving revision 1.139 > diff -u -p -r1.139 in_pcb.c > --- netinet/in_pcb.c 1 Jun 2013 13:25:40 - 1.139 > +++ netinet/in_pcb.c 5 Sep 2013 08:52:10 - > @@ -794,13 +794,10 @@ in_selectsrc(struct sockaddr_in *sin, st > if (IN_MULTICAST(sin->sin_addr.s_addr) && mopts != NULL) { > struct ifnet *ifp; > > - if (mopts->imo_multicast_ifp != NULL) { > - ifp = mopts->imo_multicast_ifp; > - TAILQ_FOREACH(ia, &in_ifaddr, ia_list) > - if (ia->ia_ifp == ifp && > - rtable_l2(rtableid) == ifp->if_rdomain) > - break; > - if (ia == 0) { > + ifp = mopts->imo_multicast_ifp; > + if (ifp != NULL && ifp->if_rdomain == rtable_l2(rtableid)) { > + IFP_TO_IA(ifp, ia); > + if (ia == NULL) { > *errorp = EADDRNOTAVAIL; > return NULL; > }
Do not reroll IFP_TO_IA
Diff below makes use of IFP_TO_IA() instead of rolling our own copy. For now there's no functional change, but I'd like to get this in so that once our multicast code can stop relying on global lists, we only need to modify the macro. ok? Index: netinet/in_pcb.c === RCS file: /home/ncvs/src/sys/netinet/in_pcb.c,v retrieving revision 1.139 diff -u -p -r1.139 in_pcb.c --- netinet/in_pcb.c1 Jun 2013 13:25:40 - 1.139 +++ netinet/in_pcb.c5 Sep 2013 08:52:10 - @@ -794,13 +794,10 @@ in_selectsrc(struct sockaddr_in *sin, st if (IN_MULTICAST(sin->sin_addr.s_addr) && mopts != NULL) { struct ifnet *ifp; - if (mopts->imo_multicast_ifp != NULL) { - ifp = mopts->imo_multicast_ifp; - TAILQ_FOREACH(ia, &in_ifaddr, ia_list) - if (ia->ia_ifp == ifp && - rtable_l2(rtableid) == ifp->if_rdomain) - break; - if (ia == 0) { + ifp = mopts->imo_multicast_ifp; + if (ifp != NULL && ifp->if_rdomain == rtable_l2(rtableid)) { + IFP_TO_IA(ifp, ia); + if (ia == NULL) { *errorp = EADDRNOTAVAIL; return NULL; }
Do not reroll IFP_TO_IA
Make use of IFP_TO_IA() in the multicast code path of ip_output() instead of rolling our own copy. No functional change. ok? Index: netinet/ip_output.c === RCS file: /home/ncvs/src/sys/netinet/ip_output.c,v retrieving revision 1.241 diff -u -p -r1.241 ip_output.c --- netinet/ip_output.c 11 Jun 2013 18:15:53 - 1.241 +++ netinet/ip_output.c 19 Jun 2013 13:46:16 - @@ -475,13 +475,9 @@ reroute: * of outgoing interface. */ if (ip->ip_src.s_addr == INADDR_ANY) { - struct in_ifaddr *ia; - - TAILQ_FOREACH(ia, &in_ifaddr, ia_list) - if (ia->ia_ifp == ifp) { - ip->ip_src = ia->ia_addr.sin_addr; - break; - } + IFP_TO_IA(ifp, ia); + if (ia != NULL) + ip->ip_src = ia->ia_addr.sin_addr; } IN_LOOKUP_MULTI(ip->ip_dst, ifp, inm);