Re: [PATCH net-next v7 06/12] net: vxlan: make vxlan_snoop() return drop reasons

[Menglong Dong]

On Sun, Oct 13, 2024 at 7:41 PM Ido Schimmel <idosch@xxxxxxxxxx> wrote:
>
> On Wed, Oct 09, 2024 at 10:28:24AM +0800, Menglong Dong wrote:
> > Change the return type of vxlan_snoop() from bool to enum
> > skb_drop_reason. In this commit, two drop reasons are introduced:
> >
> >   SKB_DROP_REASON_MAC_INVALID_SOURCE
> >   SKB_DROP_REASON_VXLAN_ENTRY_EXISTS
> >
> > Signed-off-by: Menglong Dong <dongml2@xxxxxxxxxxxxxxx>
> > Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
>
> Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx>
>
> IMO the second reason is quite obscure and unlikely to be very useful,
> but time will tell. The closest thing in the bridge driver is 802.1X /
> MAB support (see "locked" and "mab" bridge link attributes in "man
> bridge"), but I don't think it's close enough to allow us making this
> reason more generic.

Yeah, the concept of the second reason is a little obscure to
the users.

>
> [...]
>
> > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> > index 34b44755f663..1a81a3957327 100644
> > --- a/drivers/net/vxlan/vxlan_core.c
> > +++ b/drivers/net/vxlan/vxlan_core.c
> > @@ -1437,9 +1437,10 @@ static int vxlan_fdb_get(struct sk_buff *skb,
> >   * and Tunnel endpoint.
> >   * Return true if packet is bogus and should be dropped.
>
> The last line is no longer correct so please remove it in a follow up
> patch (unless you need another version).
>

Okay, I'll remove it in the next version.

Thanks!
Menglong Dong

> >   */
> > -static bool vxlan_snoop(struct net_device *dev,
> > -                     union vxlan_addr *src_ip, const u8 *src_mac,
> > -                     u32 src_ifindex, __be32 vni)
> > +static enum skb_drop_reason vxlan_snoop(struct net_device *dev,
> > +                                     union vxlan_addr *src_ip,
> > +                                     const u8 *src_mac, u32 src_ifindex,
> > +                                     __be32 vni)
> >  {
> >       struct vxlan_dev *vxlan = netdev_priv(dev);
> >       struct vxlan_fdb *f;