Re: [patch net-next 3/7] ipv4: ipmr: Don't forward packets already forwarded by hardware

2017-10-01 Thread Yotam Gigi 
On 09/28/2017 08:56 PM, Florian Fainelli wrote:
> On 09/28/2017 10:34 AM, Jiri Pirko wrote:
>> From: Yotam Gigi 
>>
>> Change the ipmr module to not forward packets if:
>>  - The packet is marked with the offload_mr_fwd_mark, and
>>  - Both input interface and output interface share the same parent ID.
>>
>> This way, a packet can go through partial multicast forwarding in the
>> hardware, where it will be forwarded only to the devices that share the
>> same parent ID (AKA, reside inside the same hardware). The kernel will
>> forward the packet to all other interfaces.
>>
>> To do this, add the ipmr_offload_forward helper, which per skb, ingress VIF
>> and egress VIF, returns whether the forwarding was offloaded to hardware.
>> The ipmr_queue_xmit frees the skb and does not forward it if the result is
>> a true value.
>>
>> All the forwarding path code compiles out when the CONFIG_NET_SWITCHDEV is
>> not set.
>>
>> Signed-off-by: Yotam Gigi 
>> Reviewed-by: Ido Schimmel 
>> Signed-off-by: Jiri Pirko 
>> ---
>>  net/ipv4/ipmr.c | 37 -
>>  1 file changed, 32 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
>> index 4566c54..deba569 100644
>> --- a/net/ipv4/ipmr.c
>> +++ b/net/ipv4/ipmr.c
>> @@ -1857,10 +1857,33 @@ static inline int ipmr_forward_finish(struct net 
>> *net, struct sock *sk,
>>  return dst_output(net, sk, skb);
>>  }
>>  
>> +#ifdef CONFIG_NET_SWITCHDEV
>> +static bool ipmr_forward_offloaded(struct sk_buff *skb, struct mr_table 
>> *mrt,
>> +   int in_vifi, int out_vifi)
>> +{
>> +struct vif_device *out_vif = &mrt->vif_table[out_vifi];
>> +struct vif_device *in_vif = &mrt->vif_table[in_vifi];
> Nit: in_vifi and out_vifi may be better named as in_vif_idx and
> out_vif_idx, oh well you are just replicating the existing naming
> conventions used down below, never mind then.


Yes, unfortunately, the acronym "vifi" is pretty common in the file. I would
also prefer something like vif_index, but vifi would better match the current
convention in the code.

Re: [patch net-next 3/7] ipv4: ipmr: Don't forward packets already forwarded by hardware

2017-09-28 Thread Florian Fainelli 
On 09/28/2017 10:34 AM, Jiri Pirko wrote:
> From: Yotam Gigi 
> 
> Change the ipmr module to not forward packets if:
>  - The packet is marked with the offload_mr_fwd_mark, and
>  - Both input interface and output interface share the same parent ID.
> 
> This way, a packet can go through partial multicast forwarding in the
> hardware, where it will be forwarded only to the devices that share the
> same parent ID (AKA, reside inside the same hardware). The kernel will
> forward the packet to all other interfaces.
> 
> To do this, add the ipmr_offload_forward helper, which per skb, ingress VIF
> and egress VIF, returns whether the forwarding was offloaded to hardware.
> The ipmr_queue_xmit frees the skb and does not forward it if the result is
> a true value.
> 
> All the forwarding path code compiles out when the CONFIG_NET_SWITCHDEV is
> not set.
> 
> Signed-off-by: Yotam Gigi 
> Reviewed-by: Ido Schimmel 
> Signed-off-by: Jiri Pirko 
> ---
>  net/ipv4/ipmr.c | 37 -
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 4566c54..deba569 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -1857,10 +1857,33 @@ static inline int ipmr_forward_finish(struct net 
> *net, struct sock *sk,
>   return dst_output(net, sk, skb);
>  }
>  
> +#ifdef CONFIG_NET_SWITCHDEV
> +static bool ipmr_forward_offloaded(struct sk_buff *skb, struct mr_table *mrt,
> +int in_vifi, int out_vifi)
> +{
> + struct vif_device *out_vif = &mrt->vif_table[out_vifi];
> + struct vif_device *in_vif = &mrt->vif_table[in_vifi];

Nit: in_vifi and out_vifi may be better named as in_vif_idx and
out_vif_idx, oh well you are just replicating the existing naming
conventions used down below, never mind then.
-- 
Florian

[patch net-next 3/7] ipv4: ipmr: Don't forward packets already forwarded by hardware

2017-09-28 Thread Jiri Pirko 
From: Yotam Gigi 

Change the ipmr module to not forward packets if:
 - The packet is marked with the offload_mr_fwd_mark, and
 - Both input interface and output interface share the same parent ID.

This way, a packet can go through partial multicast forwarding in the
hardware, where it will be forwarded only to the devices that share the
same parent ID (AKA, reside inside the same hardware). The kernel will
forward the packet to all other interfaces.

To do this, add the ipmr_offload_forward helper, which per skb, ingress VIF
and egress VIF, returns whether the forwarding was offloaded to hardware.
The ipmr_queue_xmit frees the skb and does not forward it if the result is
a true value.

All the forwarding path code compiles out when the CONFIG_NET_SWITCHDEV is
not set.

Signed-off-by: Yotam Gigi 
Reviewed-by: Ido Schimmel 
Signed-off-by: Jiri Pirko 
---
 net/ipv4/ipmr.c | 37 -
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 4566c54..deba569 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1857,10 +1857,33 @@ static inline int ipmr_forward_finish(struct net *net, 
struct sock *sk,
return dst_output(net, sk, skb);
 }
 
+#ifdef CONFIG_NET_SWITCHDEV
+static bool ipmr_forward_offloaded(struct sk_buff *skb, struct mr_table *mrt,
+  int in_vifi, int out_vifi)
+{
+   struct vif_device *out_vif = &mrt->vif_table[out_vifi];
+   struct vif_device *in_vif = &mrt->vif_table[in_vifi];
+
+   if (!skb->offload_mr_fwd_mark)
+   return false;
+   if (!out_vif->dev_parent_id_valid || !in_vif->dev_parent_id_valid)
+   return false;
+   return netdev_phys_item_id_same(&out_vif->dev_parent_id,
+   &in_vif->dev_parent_id);
+}
+#else
+static bool ipmr_forward_offloaded(struct sk_buff *skb, struct mr_table *mrt,
+  int in_vifi, int out_vifi)
+{
+   return false;
+}
+#endif
+
 /* Processing handlers for ipmr_forward */
 
 static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
-   struct sk_buff *skb, struct mfc_cache *c, int vifi)
+   int in_vifi, struct sk_buff *skb,
+   struct mfc_cache *c, int vifi)
 {
const struct iphdr *iph = ip_hdr(skb);
struct vif_device *vif = &mrt->vif_table[vifi];
@@ -1881,6 +1904,9 @@ static void ipmr_queue_xmit(struct net *net, struct 
mr_table *mrt,
goto out_free;
}
 
+   if (ipmr_forward_offloaded(skb, mrt, in_vifi, vifi))
+   goto out_free;
+
if (vif->flags & VIFF_TUNNEL) {
rt = ip_route_output_ports(net, &fl4, NULL,
   vif->remote, vif->local,
@@ -2058,8 +2084,8 @@ static void ip_mr_forward(struct net *net, struct 
mr_table *mrt,
struct sk_buff *skb2 = skb_clone(skb, 
GFP_ATOMIC);
 
if (skb2)
-   ipmr_queue_xmit(net, mrt, skb2, cache,
-   psend);
+   ipmr_queue_xmit(net, mrt, true_vifi,
+   skb2, cache, psend);
}
psend = ct;
}
@@ -2070,9 +2096,10 @@ static void ip_mr_forward(struct net *net, struct 
mr_table *mrt,
struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
 
if (skb2)
-   ipmr_queue_xmit(net, mrt, skb2, cache, psend);
+   ipmr_queue_xmit(net, mrt, true_vifi, skb2,
+   cache, psend);
} else {
-   ipmr_queue_xmit(net, mrt, skb, cache, psend);
+   ipmr_queue_xmit(net, mrt, true_vifi, skb, cache, psend);
return;
}
}
-- 
2.9.5