Re: [PATCH net-next v2 1/2] net: l3mdev: Add hook in ip and ipv6

[David Ahern <dsa@cumulusnetworks.com>]

On 5/7/16 12:32 PM, Shmulik Ladkani wrote:
> Hi David,
>
> On Sat, 7 May 2016 08:50:49 -0600 David Ahern <dsa@cumulusnetworks.com> wrote:
>>>> +static inline
>>>> +struct sk_buff *l3mdev_l3_rcv(struct sk_buff *skb, u16 proto)
>>>> +{
>>>> +	struct net_device *master = NULL;
>>>> +
>>>> +	if (netif_is_l3_slave(skb->dev))
>>>> +		master = netdev_master_upper_dev_get_rcu(skb->dev);
>>>> +
>>>> +	else if (netif_is_l3_master(skb->dev))
>>>> +		master = skb->dev;
>>>> +
>>>> +	if (master && master->l3mdev_ops->l3mdev_l3_rcv)
>>>> +		skb = master->l3mdev_ops->l3mdev_l3_rcv(master, skb, proto);
>>>
>>> In the case where netif_is_l3_master(skb->dev) is true, can you explain
>>> why we need to pass it through the l3mdev_l3_rcv callback again?
>>
>> what do you mean again? This is only time the l3mdev_l3_rcv method is
>> called on a packet.
>
> You have the following:
>
> 	if (netif_is_l3_slave(skb->dev))
> 		master = netdev_master_upper_dev_get_rcu(skb->dev);
> 	else if (netif_is_l3_master(skb->dev))
> 		master = skb->dev;
> 	if (master && master->l3mdev_ops->l3mdev_l3_rcv)
> 		skb = master->l3mdev_ops->l3mdev_l3_rcv(master, skb, proto);
>
> So in both conditions (skb->dev being the slave or the master) the skb
> is passed to master's l3mdev_l3_rcv callback.
>
> Appreciate if you can elaborate:
>   - Why callback needs to be invoked when skb->dev is the L3 master?

Every l3mdev_ops has converged on that ordering -- if l3 slave else if 
l3 master.

>   - On which circumstances we end up entering
>     l3mdev_ip_rcv/l3mdev_ip6_rcv where skb->dev is the master?
>     If I got it right, we enter 'ip_rcv_finish' on a slave device,
>     the callback is invoked and eventually sets skb->dev and skb->skb_iif
>     to the VRF device; then ip_rcv_finish continues processing the
>     altered skb (with the changed skb->dev).
>     So on which cicumstances do we enter 'ip_rcv_finish' where the
>     skb->dev is ALREADY a master device?

If you look at the full patchset I posted on 5/4 the patch after PKTINFO 
allows local traffic. That change needs the netif_is_l3_master().