[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z6IRbns62vv7eJIg@t-dallas>
Date: Tue, 4 Feb 2025 21:09:02 +0800
From: Ted Chen <znscnchen@...il.com>
To: Ido Schimmel <idosch@...sch.org>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, andrew+netdev@...n.ch, netdev@...r.kernel.org
Subject: Re: [PATCH RFC net-next 1/3] vxlan: vxlan_vs_find_vni(): Find
vxlan_dev according to vni and remote_ip
On Sun, Feb 02, 2025 at 01:56:36PM +0200, Ido Schimmel wrote:
> On Sat, Feb 01, 2025 at 07:34:00PM +0800, Ted Chen wrote:
> > vxlan_vs_find_vni() currently searches the vni hash table in a vs and
> > returns a vxlan_dev associated with the specified "vni". While this works
> > when the remote_ips are stored in the vxlan fdb, it fails to handle the
> > case where the remote_ip is just configured in the vxlan device outside of
> > the vxlan fdb, because multiple vxlan devices with different remote_ip may
> > share a single vni when the remote_ip is configured in the vxlan device
> > (i.e., not stored in the vxlan fdb). In that case, further check of
> > remote_ip to identify vxlan_dev more precisely.
> >
> > Signed-off-by: Ted Chen <znscnchen@...il.com>
> > ---
> > drivers/net/vxlan/vxlan_core.c | 32 ++++++++++++++++++++++++++------
> > 1 file changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> > index 05c10acb2a57..3ca74a97c44f 100644
> > --- a/drivers/net/vxlan/vxlan_core.c
> > +++ b/drivers/net/vxlan/vxlan_core.c
> > @@ -94,8 +94,10 @@ static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
> >
> > static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs,
> > int ifindex, __be32 vni,
> > + const struct sk_buff *skb,
> > struct vxlan_vni_node **vninode)
> > {
> > + union vxlan_addr saddr;
> > struct vxlan_vni_node *vnode;
> > struct vxlan_dev_node *node;
> >
> > @@ -116,14 +118,31 @@ static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs,
> > continue;
> > }
> >
> > - if (IS_ENABLED(CONFIG_IPV6)) {
> > - const struct vxlan_config *cfg = &node->vxlan->cfg;
> > + const struct vxlan_config *cfg = &node->vxlan->cfg;
> >
> > + if (IS_ENABLED(CONFIG_IPV6)) {
> > if ((cfg->flags & VXLAN_F_IPV6_LINKLOCAL) &&
> > cfg->remote_ifindex != ifindex)
> > continue;
> > }
> >
> > + if (vni && !vxlan_addr_any(&cfg->remote_ip) &&
> > + !vxlan_addr_multicast(&cfg->remote_ip)) {
> > + /* Get address from the outer IP header */
> > + if (vxlan_get_sk_family(vs) == AF_INET) {
> > + saddr.sin.sin_addr.s_addr = ip_hdr(skb)->saddr;
> > + saddr.sa.sa_family = AF_INET;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + } else {
> > + saddr.sin6.sin6_addr = ipv6_hdr(skb)->saddr;
> > + saddr.sa.sa_family = AF_INET6;
> > +#endif
> > + }
> > +
> > + if (!vxlan_addr_equal(&cfg->remote_ip, &saddr))
> > + continue;
>
> This breaks existing behavior. Before this patch, a VXLAN device with a
> remote address could receive traffic from any VTEP (in the same
> broadcast domain).
>
> I think this patch misinterprets the "remote" keyword as P2P when it is
> not the case. It is merely the VTEP to which packets are sent when no
Yes. Thanks for pointing that out.
I didn't see target addresses were appended into the FDB when an unicast
remote_ip had been configured.
e.g.
Usually when (2)(3) are invoked, (1) is not called to configure a unicast
remote_ip to the VTEP (though it's allowed to call (1)).
(1) ip link add vxlan42 type vxlan id 42 \
local 10.0.0.1 remote 10.0.0.2 dstport 4789
(2) bridge fdb append to 00:00:00:00:00:00 dst 10.0.0.3 dev vxlan42
(3) bridge fdb append to 00:00:00:00:00:00 dst 10.0.0.4 dev vxlan42
So, this patch just leverages the case when remote_ip is configured in the
VTEP to stand for P2P.
Do you think there's a better way to identify P2P more precisely?
> other VTEP was found in the FDB. A VXLAN device that was configured with
> the "remote" keyword can still send packets to other VTEPs and it should
> therefore be able to receive packets from them.
>
> > + }
> > +
> > if (vninode)
> > *vninode = vnode;
> > return node->vxlan;
> > @@ -134,6 +153,7 @@ static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs,
> >
> > /* Look up VNI in a per net namespace table */
> > static struct vxlan_dev *vxlan_find_vni(struct net *net, int ifindex,
> > + const struct sk_buff *skb,
> > __be32 vni, sa_family_t family,
> > __be16 port, u32 flags)
> > {
> > @@ -143,7 +163,7 @@ static struct vxlan_dev *vxlan_find_vni(struct net *net, int ifindex,
> > if (!vs)
> > return NULL;
> >
> > - return vxlan_vs_find_vni(vs, ifindex, vni, NULL);
> > + return vxlan_vs_find_vni(vs, ifindex, vni, skb, NULL);
> > }
> >
> > /* Fill in neighbour message in skbuff. */
> > @@ -1701,7 +1721,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
> >
> > vni = vxlan_vni(vh->vx_vni);
> >
> > - vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni, &vninode);
> > + vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni, skb, &vninode);
> > if (!vxlan) {
> > reason = SKB_DROP_REASON_VXLAN_VNI_NOT_FOUND;
> > goto drop;
> > @@ -1855,7 +1875,7 @@ static int vxlan_err_lookup(struct sock *sk, struct sk_buff *skb)
> > return -ENOENT;
> >
> > vni = vxlan_vni(hdr->vx_vni);
> > - vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni, NULL);
> > + vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni, skb, NULL);
> > if (!vxlan)
> > return -ENOENT;
> >
> > @@ -2330,7 +2350,7 @@ static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
> > struct vxlan_dev *dst_vxlan;
> >
> > dst_release(dst);
> > - dst_vxlan = vxlan_find_vni(vxlan->net, dst_ifindex, vni,
> > + dst_vxlan = vxlan_find_vni(vxlan->net, dst_ifindex, skb, vni,
> > addr_family, dst_port,
> > vxlan->cfg.flags);
> > if (!dst_vxlan) {
> > --
> > 2.39.2
> >
> >
Powered by blists - more mailing lists