| Message ID | 1299935679-18135-2-git-send-email-jpirko@redhat.com |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, Mar 12, 2011 at 9:14 PM, Jiri Pirko <jpirko@redhat.com> wrote: > Since skb_iif has the desired value (ifindex of physical device actually > received the traffic) use that instead. > > Signed-off-by: Jiri Pirko <jpirko@redhat.com> > Reviewed-by: Changli Gao <xiaosuo@gmail.com> > Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr> > --- This may "break" the ptype handlers in TX path, as we always assign skb->dev to origdev there. Thanks.
Mon, Mar 14, 2011 at 12:52:37AM CET, xiaosuo@gmail.com wrote: >On Sat, Mar 12, 2011 at 9:14 PM, Jiri Pirko <jpirko@redhat.com> wrote: >> Since skb_iif has the desired value (ifindex of physical device actually >> received the traffic) use that instead. >> >> Signed-off-by: Jiri Pirko <jpirko@redhat.com> >> Reviewed-by: Changli Gao <xiaosuo@gmail.com> >> Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr> >> --- > >This may "break" the ptype handlers in TX path, as we always assign >skb->dev to origdev there. Thanks. Changli, would you please point me the relevant code?. Thanks! Jirka > >-- >Regards, >Changli Gao(xiaosuo@gmail.com) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 14, 2011 at 2:54 PM, Jiri Pirko <jpirko@redhat.com> wrote: > Mon, Mar 14, 2011 at 12:52:37AM CET, xiaosuo@gmail.com wrote: >>On Sat, Mar 12, 2011 at 9:14 PM, Jiri Pirko <jpirko@redhat.com> wrote: >>> Since skb_iif has the desired value (ifindex of physical device actually >>> received the traffic) use that instead. >>> >>> Signed-off-by: Jiri Pirko <jpirko@redhat.com> >>> Reviewed-by: Changli Gao <xiaosuo@gmail.com> >>> Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr> >>> --- >> >>This may "break" the ptype handlers in TX path, as we always assign >>skb->dev to origdev there. Thanks. > > Changli, would you please point me the relevant code?. Thanks! > 1557 static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev) .... 1571 if (pt_prev) { 1572 deliver_skb(skb2, pt_prev, skb->dev); 1573 pt_prev = ptype; 1574 continue; 1575 } ... 1603 } 1604 if (pt_prev) 1605 pt_prev->func(skb2, skb->dev, pt_prev, skb->dev); 1606 rcu_read_unlock(); 1607 }
Mon, Mar 14, 2011 at 07:59:39AM CET, xiaosuo@gmail.com wrote: >On Mon, Mar 14, 2011 at 2:54 PM, Jiri Pirko <jpirko@redhat.com> wrote: >> Mon, Mar 14, 2011 at 12:52:37AM CET, xiaosuo@gmail.com wrote: >>>On Sat, Mar 12, 2011 at 9:14 PM, Jiri Pirko <jpirko@redhat.com> wrote: >>>> Since skb_iif has the desired value (ifindex of physical device actually >>>> received the traffic) use that instead. >>>> >>>> Signed-off-by: Jiri Pirko <jpirko@redhat.com> >>>> Reviewed-by: Changli Gao <xiaosuo@gmail.com> >>>> Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr> >>>> --- >>> >>>This may "break" the ptype handlers in TX path, as we always assign >>>skb->dev to origdev there. Thanks. >> >> Changli, would you please point me the relevant code?. Thanks! >> > >1557 static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev) >.... > >1571 if (pt_prev) { >1572 deliver_skb(skb2, pt_prev, skb->dev); >1573 pt_prev = ptype; >1574 continue; >1575 } >... >1603 } >1604 if (pt_prev) >1605 pt_prev->func(skb2, skb->dev, pt_prev, skb->dev); >1606 rcu_read_unlock(); >1607 } I'm probably missign something but I do not see the connection between this and setting up sll->sll_ifindex in net/packet/af_packet.c > >-- >Regards, >Changli Gao(xiaosuo@gmail.com) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 14, 2011 at 3:43 PM, Jiri Pirko <jpirko@redhat.com> wrote: > Mon, Mar 14, 2011 at 07:59:39AM CET, xiaosuo@gmail.com wrote: >>> >> >>1557 static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev) >>.... >> >>1571 if (pt_prev) { >>1572 deliver_skb(skb2, pt_prev, skb->dev); >>1573 pt_prev = ptype; >>1574 continue; >>1575 } >>... >>1603 } >>1604 if (pt_prev) >>1605 pt_prev->func(skb2, skb->dev, pt_prev, skb->dev); >>1606 rcu_read_unlock(); >>1607 } > > I'm probably missign something but I do not see the connection between > this and setting up sll->sll_ifindex in net/packet/af_packet.c > Think about a packet socket(with origdev set) for packets emitted via NIC eth0, and sll->sll_ifindex is eth0->ifindex, but after your patch, sll->sll_ifindex may be ZERO or the ifindex of the RX NIC for the forwarded packets. However, I don't know if this origdev usage is valid. Thanks.
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index b5362e9..714383c 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -627,7 +627,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev, sll->sll_protocol = skb->protocol; sll->sll_pkttype = skb->pkt_type; if (unlikely(po->origdev)) - sll->sll_ifindex = orig_dev->ifindex; + sll->sll_ifindex = skb->skb_iif; else sll->sll_ifindex = dev->ifindex; @@ -812,7 +812,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, sll->sll_protocol = skb->protocol; sll->sll_pkttype = skb->pkt_type; if (unlikely(po->origdev)) - sll->sll_ifindex = orig_dev->ifindex; + sll->sll_ifindex = skb->skb_iif; else sll->sll_ifindex = dev->ifindex;