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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d1bf0ca6-5bf9-6490-fbcd-dfc184b6ceec@mellanox.com>
Date:   Sun, 1 Oct 2017 11:51:58 +0300
From:   Yotam Gigi <yotamg@...lanox.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, idosch@...lanox.com, mlxsw@...lanox.com,
        nikolay@...ulusnetworks.com, andrew@...n.ch,
        dsa@...ulusnetworks.com, edumazet@...gle.com, willemb@...gle.com,
        johannes.berg@...el.com, dcaratti@...hat.com, pabeni@...hat.com,
        daniel@...earbox.net, fw@...len.de, gfree.wind@....163.com
Subject: Re: [patch net-next 3/7] ipv4: ipmr: Don't forward packets already
 forwarded by hardware

On 09/28/2017 08:56 PM, Florian Fainelli wrote:
> On 09/28/2017 10:34 AM, Jiri Pirko wrote:
>> From: Yotam Gigi <yotamg@...lanox.com>
>>
>> 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 <yotamg@...lanox.com>
>> Reviewed-by: Ido Schimmel <idosch@...lanox.com>
>> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
>> ---
>>  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.