Skip to contentSkip to conversation

ipvlan: read direct ifindex instead of iflink

PATCH Applied Brenden Blanco 4 participants 4 messages Oct 20, 2015 – Oct 22, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ipvlan: read direct ifindex instead of iflink

In the ipv4 outbound path of an ipvlan device in l3 mode, the ifindex is being grabbed from dev_get_iflink. This works for the physical device case, since as the documentation of that function notes: "Physical interfaces have the same 'ifindex' and 'iflink' values.". However, if the master device is a veth, and the pairs are in separate net namespaces, the route lookup will fail with -ENODEV due to outer veth pair being in a separate namespace from the ipvlan master/routing namespace.

  ns0    |   ns1    |   ns2
 veth0a--|--veth0b--|--ipvl0

In ipvlan_process_v4_outbound(), a packet sent from ipvl0 in the above configuration will pass fl.flowi4_oif == veth0a to ip_route_output_flow(), but *net == ns1.

Notice also that ipv6 processing is not using iflink. Since there is a discrepancy in usage, fixup both v4 and v6 case to use local dev variable.

Tested this with l3 ipvlan on top of veth, as well as with single physical interface in the top namespace.

---
 drivers/net/ipvlan/ipvlan_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ipvlan/ipvlan_core.c +2 -2
index 24f8dbc..d50887e 100644
@@ -348,7 +348,7 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
 	struct rtable *rt;
 	int err, ret = NET_XMIT_DROP;
 	struct flowi4 fl4 = {
-		.flowi4_oif = dev_get_iflink(dev),
+		.flowi4_oif = dev->ifindex,
 		.flowi4_tos = RT_TOS(ip4h->tos),
 		.flowi4_flags = FLOWI_FLAG_ANYSRC,
 		.daddr = ip4h->daddr,
@@ -386,7 +386,7 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
 	struct dst_entry *dst;
 	int err, ret = NET_XMIT_DROP;
 	struct flowi6 fl6 = {
-		.flowi6_iif = skb->dev->ifindex,
+		.flowi6_iif = dev->ifindex,
 		.daddr = ip6h->daddr,
 		.saddr = ip6h->saddr,
 		.flowi6_flags = FLOWI_FLAG_ANYSRC,
Signature · 1 line
2.6.1
Reviewed-by on v1 1/1
#1 lore raw

drivers/net/ipvlan/ipvlan_core.c static int ipvlan_process_v4_outbound(struct sk_buff *skb) · line 351

Quoted 8 lines · Brenden Blanco
drivers/net/ipvlan/ipvlan_core.c +1 -1
@@ -348,7 +348,7 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
 	struct rtable *rt;
 	int err, ret = NET_XMIT_DROP;
 	struct flowi4 fl4 = {
-		.flowi4_oif = dev_get_iflink(dev),
+		.flowi4_oif = dev->ifindex,

The current code is obviously wrong and the patch makes sense.

Signature · 1 line
Jiri Benc
Acked-by on v1 1/1
#2 lore raw
In reply to Brenden Blanco
Quoted 9 lines · Brenden Blanco

In the ipv4 outbound path of an ipvlan device in l3 mode, the ifindex is being grabbed from dev_get_iflink. This works for the physical device case, since as the documentation of that function notes: "Physical interfaces have the same 'ifindex' and 'iflink' values.". However, if the master device is a veth, and the pairs are in separate net namespaces, the route lookup will fail with -ENODEV due to outer veth pair being in a separate namespace from the ipvlan master/routing namespace.

Quoted 3 lines
  ns0    |   ns1    |   ns2
 veth0a--|--veth0b--|--ipvl0

interesting use case.

Quoted 4 lines

In ipvlan_process_v4_outbound(), a packet sent from ipvl0 in the above configuration will pass fl.flowi4_oif == veth0a to ip_route_output_flow(), but *net == ns1.

Quoted 8 lines

Notice also that ipv6 processing is not using iflink. Since there is a discrepancy in usage, fixup both v4 and v6 case to use local dev variable.

Tested this with l3 ipvlan on top of veth, as well as with single physical interface in the top namespace.

Quoted 29 lines
---
 drivers/net/ipvlan/ipvlan_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ipvlan/ipvlan_core.c +2 -3
index 24f8dbc..d50887e 100644
@@ -348,7 +348,7 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
        struct rtable *rt;
        int err, ret = NET_XMIT_DROP;
        struct flowi4 fl4 = {
-               .flowi4_oif = dev_get_iflink(dev),
+               .flowi4_oif = dev->ifindex,
                .flowi4_tos = RT_TOS(ip4h->tos),
                .flowi4_flags = FLOWI_FLAG_ANYSRC,
                .daddr = ip4h->daddr,
@@ -386,7 +386,7 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
        struct dst_entry *dst;
        int err, ret = NET_XMIT_DROP;
        struct flowi6 fl6 = {
-               .flowi6_iif = skb->dev->ifindex,
+               .flowi6_iif = dev->ifindex,
                .daddr = ip6h->daddr,
                .saddr = ip6h->saddr,
                .flowi6_flags = FLOWI_FLAG_ANYSRC,
--

2.6.1

maintainer Applied
#3 lore raw
In reply to Brenden Blanco

From: Brenden Blanco <[email protected]>
Date: Tue, 20 Oct 2015 16:47:33 -0700

Quoted 16 lines

In the ipv4 outbound path of an ipvlan device in l3 mode, the ifindex is being grabbed from dev_get_iflink. This works for the physical device case, since as the documentation of that function notes: "Physical interfaces have the same 'ifindex' and 'iflink' values.". However, if the master device is a veth, and the pairs are in separate net namespaces, the route lookup will fail with -ENODEV due to outer veth pair being in a separate namespace from the ipvlan master/routing namespace.

  ns0    |   ns1    |   ns2
 veth0a--|--veth0b--|--ipvl0

In ipvlan_process_v4_outbound(), a packet sent from ipvl0 in the above configuration will pass fl.flowi4_oif == veth0a to ip_route_output_flow(), but *net == ns1.

Quoted 8 lines

Notice also that ipv6 processing is not using iflink. Since there is a discrepancy in usage, fixup both v4 and v6 case to use local dev variable.

Tested this with l3 ipvlan on top of veth, as well as with single physical interface in the top namespace.

Applied, thanks Brenden.