* [PATCH] core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()
@ 2013-07-02 11:30 Isaku Yamahata
2013-07-02 23:00 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Isaku Yamahata @ 2013-07-02 11:30 UTC (permalink / raw)
To: netdev
Cc: yamahata, murphy.mccauley, Jason Wang, Michael S. Tsirkin,
Eric Dumazet, Patrick McHardy, Hong Zhiguo, Rami Rosen,
Tom Parkin, Cong Wang, Pravin B Shelar, Jesse Gross, dev
The dev_forward_skb() assignment of pkt_type should be done
after the call to eth_type_trans().
ip-encapsulated packets can be handled by localhost. But skb->pkt_type
can be PACKET_OTHERHOST when packet comes via veth into ip tunnel device.
In that case, the packet is dropped by ip_rcv().
Although this example uses gretap. l2tp-eth also has same issue.
For l2tp-eth case, add dummy device for ip address and ip l2tp command.
netns A | root netns | netns B
veth<->veth=bridge=gretap <-loop back-> gretap=bridge=veth<->veth
arp packet ->
pkt_type
BROADCAST------------>ip_rcv()------------------------>
<- arp reply
pkt_type
ip_rcv()<-----------------OTHERHOST
drop
sample operations
ip link add tapa type gretap remote 172.17.107.4 local 172.17.107.3
ip link add tapb type gretap remote 172.17.107.3 local 172.17.107.4
ip link set tapa up
ip link set tapb up
ip address add 172.17.107.3 dev tapa
ip address add 172.17.107.4 dev tapb
ip route get 172.17.107.3
> local 172.17.107.3 dev lo src 172.17.107.3
> cache <local>
ip route get 172.17.107.4
> local 172.17.107.4 dev lo src 172.17.107.4
> cache <local>
ip link add vetha type veth peer name vetha-peer
ip link add vethb type veth peer name vethb-peer
brctl addbr bra
brctl addbr brb
brctl addif bra tapa
brctl addif bra vetha-peer
brctl addif brb tapb
brctl addif brb vethb-peer
brctl show
> bridge name bridge id STP enabled interfaces
> bra 8000.6ea21e758ff1 no tapa
> vetha-peer
> brb 8000.420020eb92d5 no tapb
> vethb-peer
ip link set vetha-peer up
ip link set vethb-peer up
ip link set bra up
ip link set brb up
ip netns add a
ip netns add b
ip link set vetha netns a
ip link set vethb netns b
ip netns exec a ip address add 10.0.0.3/24 dev vetha
ip netns exec b ip address add 10.0.0.4/24 dev vethb
ip netns exec a ip link set vetha up
ip netns exec b ip link set vethb up
ip netns exec a arping -I vetha 10.0.0.4
ARPING 10.0.0.4 from 10.0.0.3 vetha
^CSent 2 probes (2 broadcast(s))
Received 0 response(s)
Cc: Jason Wang <jasowang@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Hong Zhiguo <honkiko@gmail.com>
Cc: Rami Rosen <ramirose@gmail.com>
Cc: Tom Parkin <tparkin@katalix.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Pravin B Shelar <pshelar@nicira.com>
Cc: Jesse Gross <jesse@nicira.com>
Cc: dev@openvswitch.org
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
net/core/dev.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 722f633..b179b8a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1662,8 +1662,12 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
skb->skb_iif = 0;
skb_dst_drop(skb);
skb->tstamp.tv64 = 0;
- skb->pkt_type = PACKET_HOST;
skb->protocol = eth_type_trans(skb, dev);
+ /*
+ * eth_type_trans() can set pkt_type.
+ * clear pkt_type _after_ calling eth_type_trans()
+ */
+ skb->pkt_type = PACKET_HOST;
skb->mark = 0;
secpath_reset(skb);
nf_reset(skb);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()
2013-07-02 11:30 [PATCH] core/dev: set pkt_type after eth_type_trans() in dev_forward_skb() Isaku Yamahata
@ 2013-07-02 23:00 ` David Miller
2013-07-03 14:53 ` Nicolas Dichtel
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2013-07-02 23:00 UTC (permalink / raw)
To: yamahata
Cc: netdev, murphy.mccauley, jasowang, mst, edumazet, kaber, honkiko,
ramirose, tparkin, xiyou.wangcong, pshelar, jesse, dev
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Tue, 2 Jul 2013 20:30:10 +0900
> The dev_forward_skb() assignment of pkt_type should be done
> after the call to eth_type_trans().
>
> ip-encapsulated packets can be handled by localhost. But skb->pkt_type
> can be PACKET_OTHERHOST when packet comes via veth into ip tunnel device.
> In that case, the packet is dropped by ip_rcv().
> Although this example uses gretap. l2tp-eth also has same issue.
> For l2tp-eth case, add dummy device for ip address and ip l2tp command.
...
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Applied, but I had to adjust the patch because in net-next we use
a new helper function (skb_scrub_packet()) to clear things out from
dev_forward_skb().
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()
2013-07-02 23:00 ` David Miller
@ 2013-07-03 14:53 ` Nicolas Dichtel
2013-07-04 21:57 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Dichtel @ 2013-07-03 14:53 UTC (permalink / raw)
To: David Miller
Cc: yamahata, netdev, murphy.mccauley, jasowang, mst, edumazet, kaber,
honkiko, ramirose, tparkin, xiyou.wangcong, pshelar, jesse, dev
Le 03/07/2013 01:00, David Miller a écrit :
> From: Isaku Yamahata <yamahata@valinux.co.jp>
> Date: Tue, 2 Jul 2013 20:30:10 +0900
>
>> The dev_forward_skb() assignment of pkt_type should be done
>> after the call to eth_type_trans().
>>
>> ip-encapsulated packets can be handled by localhost. But skb->pkt_type
>> can be PACKET_OTHERHOST when packet comes via veth into ip tunnel device.
>> In that case, the packet is dropped by ip_rcv().
>> Although this example uses gretap. l2tp-eth also has same issue.
>> For l2tp-eth case, add dummy device for ip address and ip l2tp command.
> ...
>> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> Applied, but I had to adjust the patch because in net-next we use
> a new helper function (skb_scrub_packet()) to clear things out from
> dev_forward_skb().
What about calling skb_scrub_packet() after eth_type_trans()?
I wonder if the same problem may happen the day gre will support x-netns,
because skb_scrub_packet() is also called before eth_type_trans() in
ip_tunnel_rcv().
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()
2013-07-03 14:53 ` Nicolas Dichtel
@ 2013-07-04 21:57 ` David Miller
2013-07-05 7:48 ` Nicolas Dichtel
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2013-07-04 21:57 UTC (permalink / raw)
To: nicolas.dichtel
Cc: yamahata, netdev, murphy.mccauley, jasowang, mst, edumazet, kaber,
honkiko, ramirose, tparkin, xiyou.wangcong, pshelar, jesse, dev
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed, 03 Jul 2013 16:53:52 +0200
> Le 03/07/2013 01:00, David Miller a écrit :
>> From: Isaku Yamahata <yamahata@valinux.co.jp>
>> Date: Tue, 2 Jul 2013 20:30:10 +0900
>>
>>> The dev_forward_skb() assignment of pkt_type should be done
>>> after the call to eth_type_trans().
>>>
>>> ip-encapsulated packets can be handled by localhost. But skb->pkt_type
>>> can be PACKET_OTHERHOST when packet comes via veth into ip tunnel
>>> device.
>>> In that case, the packet is dropped by ip_rcv().
>>> Although this example uses gretap. l2tp-eth also has same issue.
>>> For l2tp-eth case, add dummy device for ip address and ip l2tp
>>> command.
>> ...
>>> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>>
>> Applied, but I had to adjust the patch because in net-next we use
>> a new helper function (skb_scrub_packet()) to clear things out from
>> dev_forward_skb().
> What about calling skb_scrub_packet() after eth_type_trans()?
>
> I wonder if the same problem may happen the day gre will support
> x-netns,
> because skb_scrub_packet() is also called before eth_type_trans() in
> ip_tunnel_rcv().
That's a fine suggestion, I was just being overly conservative in my
forward-porting of his patch.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()
2013-07-04 21:57 ` David Miller
@ 2013-07-05 7:48 ` Nicolas Dichtel
0 siblings, 0 replies; 5+ messages in thread
From: Nicolas Dichtel @ 2013-07-05 7:48 UTC (permalink / raw)
To: David Miller
Cc: yamahata, netdev, murphy.mccauley, jasowang, mst, edumazet, kaber,
honkiko, ramirose, tparkin, xiyou.wangcong, pshelar, jesse, dev
Le 04/07/2013 23:57, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Wed, 03 Jul 2013 16:53:52 +0200
>
>> Le 03/07/2013 01:00, David Miller a écrit :
>>> From: Isaku Yamahata <yamahata@valinux.co.jp>
>>> Date: Tue, 2 Jul 2013 20:30:10 +0900
>>>
>>>> The dev_forward_skb() assignment of pkt_type should be done
>>>> after the call to eth_type_trans().
>>>>
>>>> ip-encapsulated packets can be handled by localhost. But skb->pkt_type
>>>> can be PACKET_OTHERHOST when packet comes via veth into ip tunnel
>>>> device.
>>>> In that case, the packet is dropped by ip_rcv().
>>>> Although this example uses gretap. l2tp-eth also has same issue.
>>>> For l2tp-eth case, add dummy device for ip address and ip l2tp
>>>> command.
>>> ...
>>>> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>>>
>>> Applied, but I had to adjust the patch because in net-next we use
>>> a new helper function (skb_scrub_packet()) to clear things out from
>>> dev_forward_skb().
>> What about calling skb_scrub_packet() after eth_type_trans()?
>>
>> I wonder if the same problem may happen the day gre will support
>> x-netns,
>> because skb_scrub_packet() is also called before eth_type_trans() in
>> ip_tunnel_rcv().
>
> That's a fine suggestion, I was just being overly conservative in my
> forward-porting of his patch.
>
Ok, I will wait that net-next opens to submit this patch.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-07-05 7:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-02 11:30 [PATCH] core/dev: set pkt_type after eth_type_trans() in dev_forward_skb() Isaku Yamahata
2013-07-02 23:00 ` David Miller
2013-07-03 14:53 ` Nicolas Dichtel
2013-07-04 21:57 ` David Miller
2013-07-05 7:48 ` Nicolas Dichtel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).