Patchwork [net-next-2.6,1/6] af_packet: use skb->skb_iif instead of orig_dev->ifindex

login
register
mail settings
Submitter Jiri Pirko
Date March 12, 2011, 1:14 p.m.
Message ID <1299935679-18135-2-git-send-email-jpirko@redhat.com>
Download mbox | patch
Permalink /patch/86515/
State Deferred
Delegated to: David Miller
Headers show

Comments

Jiri Pirko - March 12, 2011, 1:14 p.m.
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>
---
 net/packet/af_packet.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Changli Gao - March 13, 2011, 11:52 p.m.
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.
Jiri Pirko - March 14, 2011, 6:54 a.m.
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
Changli Gao - March 14, 2011, 6:59 a.m.
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 }
Jiri Pirko - March 14, 2011, 7:43 a.m.
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
Changli Gao - March 14, 2011, 3:10 p.m.
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.

Patch

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;