Re: [PATCH net-next v2 01/12] net: vxlan: add vxlan to the drop reason subsystem

[Menglong Dong]

On Fri, Aug 30, 2024 at 10:43 PM Alexander Lobakin
<aleksander.lobakin@xxxxxxxxx> wrote:
>
> From: Menglong Dong <menglong8.dong@xxxxxxxxx>
> Date: Fri, 30 Aug 2024 09:59:50 +0800
>
> > In this commit, we introduce the SKB_DROP_REASON_SUBSYS_VXLAN to make the
> > vxlan support skb drop reasons.
> >
> > Signed-off-by: Menglong Dong <dongml2@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/net/vxlan/drop.h          | 25 +++++++++++++++++++++++++
> >  drivers/net/vxlan/vxlan_core.c    | 15 +++++++++++++++
> >  drivers/net/vxlan/vxlan_private.h |  1 +
> >  include/net/dropreason.h          |  6 ++++++
> >  4 files changed, 47 insertions(+)
> >  create mode 100644 drivers/net/vxlan/drop.h
> >
> > diff --git a/drivers/net/vxlan/drop.h b/drivers/net/vxlan/drop.h
> > new file mode 100644
> > index 000000000000..6bcc6894fbbd
> > --- /dev/null
> > +++ b/drivers/net/vxlan/drop.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * VXLAN drop reason list.
> > + */
> > +
> > +#ifndef VXLAN_DROP_H
> > +#define VXLAN_DROP_H
>
> Empty newline here please after the guard.
>
> > +#include <linux/skbuff.h>
> > +#include <net/dropreason.h>
> > +
> > +#define VXLAN_DROP_REASONS(R)                        \
> > +     /* deliberate comment for trailing \ */
> > +
> > +enum vxlan_drop_reason {
> > +     __VXLAN_DROP_REASON = SKB_DROP_REASON_SUBSYS_VXLAN <<
> > +                             SKB_DROP_REASON_SUBSYS_SHIFT,
>
> Maybe make SHIFT start at the same position as VXLAN, i.e. align the
> former to the latter?
>

Yeah, that looks better.

> [...]
>
> > diff --git a/drivers/net/vxlan/vxlan_private.h b/drivers/net/vxlan/vxlan_private.h
> > index b35d96b78843..8720d7a1206f 100644
> > --- a/drivers/net/vxlan/vxlan_private.h
> > +++ b/drivers/net/vxlan/vxlan_private.h
> > @@ -8,6 +8,7 @@
> >  #define _VXLAN_PRIVATE_H
> >
> >  #include <linux/rhashtable.h>
>
> Also an empty newline here.
>
> > +#include "drop.h"
> >
> >  extern unsigned int vxlan_net_id;
> >  extern const u8 all_zeros_mac[ETH_ALEN + 2];
> > diff --git a/include/net/dropreason.h b/include/net/dropreason.h
> > index 56cb7be92244..2e5d158d670e 100644
> > --- a/include/net/dropreason.h
> > +++ b/include/net/dropreason.h
> > @@ -29,6 +29,12 @@ enum skb_drop_reason_subsys {
> >        */
> >       SKB_DROP_REASON_SUBSYS_OPENVSWITCH,
> >
> > +     /**
> > +      * @SKB_DROP_REASON_SUBSYS_VXLAN: vxlan drop reason, see
>
> "VXLAN", uppercase?
>

Okay! I'll change this commit as you advised.

Thanks!
Menglong Dong

> > +      * drivers/net/vxlan/drop.h
> > +      */
> > +     SKB_DROP_REASON_SUBSYS_VXLAN,
> > +
> >       /** @SKB_DROP_REASON_SUBSYS_NUM: number of subsystems defined */
> >       SKB_DROP_REASON_SUBSYS_NUM
> >  };
>
> Thanks,
> Olek