Le 14 nov. 2018 à 20:58, David Ahern <d...@cumulusnetworks.com> a écrit :
>
> you are making this more specific than it needs to be ....
>
> On 11/14/18 1:31 AM, Alexis Bauvin wrote:
>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>> index 27bd586b94b0..7477b5510a04 100644
>> --- a/drivers/net/vxlan.c
>> +++ b/drivers/net/vxlan.c
>> @@ -208,11 +208,23 @@ static inline struct vxlan_rdst
>> *first_remote_rtnl(struct vxlan_fdb *fdb)
>> return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
>> }
>>
>> +static int vxlan_get_l3mdev(struct net *net, int ifindex)
>> +{
>> + struct net_device *dev;
>> +
>> + dev = __dev_get_by_index(net, ifindex);
>> + while (dev && !netif_is_l3_master(dev))
>> + dev = netdev_master_upper_dev_get(dev);
>> +
>> + return dev ? dev->ifindex : 0;
>> +}
>
> l3mdev_master_ifindex_by_index should work instead of defining this for
> vxlan.
>
> But I do not believe you need this function.
l3mdev_master_ifindex_by_index does not recursively climbs up the master chain.
This means that if the l3mdev is not a direct master of the device, it will not
be found.
E.G. Calling l3mdev_master_ifindex_by_index with the index of eth0 will
return 0:
+------+ +-----+ +----------+
| | | | | |
| eth0 +-----+ br0 +-----+ vrf-blue |
| | | | | |
+------+ +-----+ +----------+
This is because the underlying l3mdev_master_dev_rcu function fetches the master
(br0 in this case), checks whether it is an l3mdev (which it is not), and
returns its index if so.
So if using l3mdev_master_dev_rcu, using eth0 as a lower device will still bind
to no specific device, thus in the default VRF.
Maybe I should have patched l3mdev_master_dev_rcu to do a recursive resolution
(as vxlan_get_l3mdev does), but I don’t know the impact of such a change.
>> +
>> /* Find VXLAN socket based on network namespace, address family and UDP port
>> * and enabled unshareable flags.
>> */
>> static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t
>> family,
>> - __be16 port, u32 flags)
>> + __be16 port, u32 flags,
>> + int l3mdev_ifindex)
>> {
>> struct vxlan_sock *vs;
>>
>> @@ -221,7 +233,8 @@ static struct vxlan_sock *vxlan_find_sock(struct net
>> *net, sa_family_t family,
>> hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
>> if (inet_sk(vs->sock->sk)->inet_sport == port &&
>> vxlan_get_sk_family(vs) == family &&
>> - vs->flags == flags)
>> + vs->flags == flags &&
>> + vs->sock->sk->sk_bound_dev_if == l3mdev_ifindex)
>
> Why not allow the vxlan socket to bind to any ifindex? In that case this
> socket lookup follows what we do for tcp, udp and raw sockets, and you
> don't need to call out vrf / l3mdev directly (ie.,
> s/l3mdev_ifindex/ifindex/g) - it comes for free.
Reword will be done in next version!
