* [PATCH net v2] net: ipmr: Fix some mroute forwarding issues in vrf's
@ 2017-06-09 14:22 Donald Sharp
2017-06-09 14:28 ` Andrew Lunn
2017-06-09 14:54 ` David Ahern
0 siblings, 2 replies; 4+ messages in thread
From: Donald Sharp @ 2017-06-09 14:22 UTC (permalink / raw)
To: netdev, dsahern, Thomas.Winter, nikolay, yotamg, idosch, roopa
This patch fixes two issues:
1) When forwarding on *,G mroutes that are in a vrf, the
kernel was dropping information about the actual incoming
interface when calling ip_mr_forward from ip_mr_input.
This caused ip_mr_forward to send the multicast packet
back out the incoming interface. Fix this by
modifying ip_mr_forward to be handed the correctly
resolved dev.
2) When a unresolved cache entry is created we store
the incoming skb on the unresolved cache entry and
upon mroute resolution from the user space daemon,
we attempt to forward the packet. Again we were
not resolving to the correct incoming device for
a vrf scenario, before calling ip_mr_forward.
Fix this by resolving to the correct interface
and calling ip_mr_forward with the result.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
v2: Fixed title
net/ipv4/ipmr.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 551de4d..559009e 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -101,8 +101,8 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id);
static void ipmr_free_table(struct mr_table *mrt);
static void ip_mr_forward(struct net *net, struct mr_table *mrt,
- struct sk_buff *skb, struct mfc_cache *cache,
- int local);
+ struct net_device *dev, struct sk_buff *skb,
+ struct mfc_cache *cache, int local);
static int ipmr_cache_report(struct mr_table *mrt,
struct sk_buff *pkt, vifi_t vifi, int assert);
static int __ipmr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
@@ -988,7 +988,16 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
} else {
- ip_mr_forward(net, mrt, skb, c, 0);
+ struct net_device *dev = skb->dev;
+
+ if (netif_is_l3_master(dev)) {
+ dev = __dev_get_by_index(net, IPCB(skb)->iif);
+ if (!dev) {
+ kfree_skb(skb);
+ continue;
+ }
+ }
+ ip_mr_forward(net, mrt, dev, skb, c, 0);
}
}
}
@@ -1828,10 +1837,10 @@ static int ipmr_find_vif(struct mr_table *mrt, struct net_device *dev)
/* "local" means that we should preserve one skb (for local delivery) */
static void ip_mr_forward(struct net *net, struct mr_table *mrt,
- struct sk_buff *skb, struct mfc_cache *cache,
- int local)
+ struct net_device *dev, struct sk_buff *skb,
+ struct mfc_cache *cache, int local)
{
- int true_vifi = ipmr_find_vif(mrt, skb->dev);
+ int true_vifi = ipmr_find_vif(mrt, dev);
int psend = -1;
int vif, ct;
@@ -1853,11 +1862,11 @@ static void ip_mr_forward(struct net *net, struct mr_table *mrt,
}
/* Wrong interface: drop packet and (maybe) send PIM assert. */
- if (mrt->vif_table[vif].dev != skb->dev) {
+ if (mrt->vif_table[vif].dev != dev) {
struct net_device *mdev;
mdev = l3mdev_master_dev_rcu(mrt->vif_table[vif].dev);
- if (mdev == skb->dev)
+ if (mdev == dev)
goto forward;
if (rt_is_output_route(skb_rtable(skb))) {
@@ -2064,7 +2073,7 @@ int ip_mr_input(struct sk_buff *skb)
}
read_lock(&mrt_lock);
- ip_mr_forward(net, mrt, skb, cache, local);
+ ip_mr_forward(net, mrt, dev, skb, cache, local);
read_unlock(&mrt_lock);
if (local)
--
2.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net v2] net: ipmr: Fix some mroute forwarding issues in vrf's
2017-06-09 14:22 [PATCH net v2] net: ipmr: Fix some mroute forwarding issues in vrf's Donald Sharp
@ 2017-06-09 14:28 ` Andrew Lunn
2017-06-09 14:54 ` David Ahern
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Lunn @ 2017-06-09 14:28 UTC (permalink / raw)
To: Donald Sharp
Cc: netdev, dsahern, Thomas.Winter, nikolay, yotamg, idosch, roopa
> static void ip_mr_forward(struct net *net, struct mr_table *mrt,
> - struct sk_buff *skb, struct mfc_cache *cache,
> - int local);
> + struct net_device *dev, struct sk_buff *skb,
> + struct mfc_cache *cache, int local);
You appear to have spaces here, not tabs.
Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v2] net: ipmr: Fix some mroute forwarding issues in vrf's
2017-06-09 14:22 [PATCH net v2] net: ipmr: Fix some mroute forwarding issues in vrf's Donald Sharp
2017-06-09 14:28 ` Andrew Lunn
@ 2017-06-09 14:54 ` David Ahern
2017-06-09 15:20 ` Donald Sharp
1 sibling, 1 reply; 4+ messages in thread
From: David Ahern @ 2017-06-09 14:54 UTC (permalink / raw)
To: Donald Sharp, netdev, Thomas.Winter, nikolay, yotamg, idosch,
roopa
On 6/9/17 8:22 AM, Donald Sharp wrote:
> @@ -988,7 +988,16 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
>
> rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
> } else {
> - ip_mr_forward(net, mrt, skb, c, 0);
> + struct net_device *dev = skb->dev;
> +
> + if (netif_is_l3_master(dev)) {
> + dev = __dev_get_by_index(net, IPCB(skb)->iif);
> + if (!dev) {
> + kfree_skb(skb);
> + continue;
> + }
> + }
> + ip_mr_forward(net, mrt, dev, skb, c, 0);
> }
> }
> }
What about changing ipmr_cache_unresolved to take the dev it looked up
already and then have ipmr_cache_unresolved reset skb->dev to it (and
reset skb->skb_iff to dev->ifindex) when queuing to the unresolved list?
Since this path does not have a local delivery, resetting the skb->dev
will be fine and it avoids this second lookup using IPCB(skb)->iif:
@@ -1073,7 +1073,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
/* Queue a packet for resolution. It gets locked cache entry! */
static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
- struct sk_buff *skb)
+ struct sk_buff *skb, struct net_device
*dev)
{
const struct iphdr *iph = ip_hdr(skb);
struct mfc_cache *c;
@@ -1130,6 +1130,10 @@ static int ipmr_cache_unresolved(struct mr_table
*mrt, vifi_t vifi,
kfree_skb(skb);
err = -ENOBUFS;
} else {
+ if (dev) {
+ skb->dev = dev;
+ skb->skb_iif = dev->ifindex;
+ }
skb_queue_tail(&c->mfc_un.unres.unresolved, skb);
err = 0;
}
Combined with Thomas' earlier change this check in ip_mr_forward becomes:
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 9374b99c7c17..1393a4d18a9a 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1853,13 +1853,7 @@ static void ip_mr_forward(struct net *net, struct
mr_table *mrt,
}
/* Wrong interface: drop packet and (maybe) send PIM assert. */
- if (mrt->vif_table[vif].dev != skb->dev) {
- struct net_device *mdev;
-
- mdev = l3mdev_master_dev_rcu(mrt->vif_table[vif].dev);
- if (mdev == skb->dev)
- goto forward;
-
+ if (mrt->vif_table[vif].dev != dev) {
if (rt_is_output_route(skb_rtable(skb))) {
/* It is our own packet, looped back.
* Very complicated situation...
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net v2] net: ipmr: Fix some mroute forwarding issues in vrf's
2017-06-09 14:54 ` David Ahern
@ 2017-06-09 15:20 ` Donald Sharp
0 siblings, 0 replies; 4+ messages in thread
From: Donald Sharp @ 2017-06-09 15:20 UTC (permalink / raw)
To: David Ahern
Cc: netdev, Thomas Winter, Nikolay Aleksandrov, Yotam Gigi,
Ido Schimmel, Roopa Prabhu
I'll change it over to this way. No problem.
donald
On Fri, Jun 9, 2017 at 10:54 AM, David Ahern <dsahern@gmail.com> wrote:
> On 6/9/17 8:22 AM, Donald Sharp wrote:
>> @@ -988,7 +988,16 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
>>
>> rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
>> } else {
>> - ip_mr_forward(net, mrt, skb, c, 0);
>> + struct net_device *dev = skb->dev;
>> +
>> + if (netif_is_l3_master(dev)) {
>> + dev = __dev_get_by_index(net, IPCB(skb)->iif);
>> + if (!dev) {
>> + kfree_skb(skb);
>> + continue;
>> + }
>> + }
>> + ip_mr_forward(net, mrt, dev, skb, c, 0);
>> }
>> }
>> }
>
> What about changing ipmr_cache_unresolved to take the dev it looked up
> already and then have ipmr_cache_unresolved reset skb->dev to it (and
> reset skb->skb_iff to dev->ifindex) when queuing to the unresolved list?
> Since this path does not have a local delivery, resetting the skb->dev
> will be fine and it avoids this second lookup using IPCB(skb)->iif:
>
> @@ -1073,7 +1073,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
>
> /* Queue a packet for resolution. It gets locked cache entry! */
> static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
> - struct sk_buff *skb)
> + struct sk_buff *skb, struct net_device
> *dev)
> {
> const struct iphdr *iph = ip_hdr(skb);
> struct mfc_cache *c;
> @@ -1130,6 +1130,10 @@ static int ipmr_cache_unresolved(struct mr_table
> *mrt, vifi_t vifi,
> kfree_skb(skb);
> err = -ENOBUFS;
> } else {
> + if (dev) {
> + skb->dev = dev;
> + skb->skb_iif = dev->ifindex;
> + }
> skb_queue_tail(&c->mfc_un.unres.unresolved, skb);
> err = 0;
> }
>
>
> Combined with Thomas' earlier change this check in ip_mr_forward becomes:
>
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 9374b99c7c17..1393a4d18a9a 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -1853,13 +1853,7 @@ static void ip_mr_forward(struct net *net, struct
> mr_table *mrt,
> }
>
> /* Wrong interface: drop packet and (maybe) send PIM assert. */
> - if (mrt->vif_table[vif].dev != skb->dev) {
> - struct net_device *mdev;
> -
> - mdev = l3mdev_master_dev_rcu(mrt->vif_table[vif].dev);
> - if (mdev == skb->dev)
> - goto forward;
> -
> + if (mrt->vif_table[vif].dev != dev) {
> if (rt_is_output_route(skb_rtable(skb))) {
> /* It is our own packet, looped back.
> * Very complicated situation...
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-06-09 15:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-09 14:22 [PATCH net v2] net: ipmr: Fix some mroute forwarding issues in vrf's Donald Sharp
2017-06-09 14:28 ` Andrew Lunn
2017-06-09 14:54 ` David Ahern
2017-06-09 15:20 ` Donald Sharp
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).