Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
To:     Horatiu Vultur <horatiu.vultur@microchip.com>
Cc:     Alexander Lobakin <alexandr.lobakin@intel.com>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        bpf@vger.kernel.org, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, ast@kernel.org,
        daniel@iogearbox.net, hawk@kernel.org, john.fastabend@gmail.com,
        linux@armlinux.org.uk
Subject: Re: [PATCH net-next v2 3/4] net: lan966x: Add basic XDP support
Date:   Tue,  8 Nov 2022 12:26:01 +0100
Message-Id: <20221108112601.605326-1-alexandr.lobakin@intel.com>
In-Reply-To: <20221107212618.73aqn3cdqojs6zbo@soft-dev3-1>
References: <20221106211154.3225784-1-horatiu.vultur@microchip.com> <20221106211154.3225784-4-horatiu.vultur@microchip.com> <20221107161357.556549-1-alexandr.lobakin@intel.com> <20221107212618.73aqn3cdqojs6zbo@soft-dev3-1>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Horatiu Vultur <horatiu.vultur@microchip.com>
Date: Mon, 7 Nov 2022 22:26:18 +0100

> The 11/07/2022 17:13, Alexander Lobakin wrote:
> 
> Hi Olek,
> 
> > 
> > From: Alexander Lobakin <alexander.lobakin@intel.com>
> > 
> > From: Horatiu Vultur <horatiu.vultur@microchip.com>
> > Date: Sun, 6 Nov 2022 22:11:53 +0100
> > 
> > > Introduce basic XDP support to lan966x driver. Currently the driver
> > > supports only the actions XDP_PASS, XDP_DROP and XDP_ABORTED.
> > >
> > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > > ---
> > >  .../net/ethernet/microchip/lan966x/Makefile   |  3 +-
> > >  .../ethernet/microchip/lan966x/lan966x_fdma.c | 11 ++-
> > >  .../ethernet/microchip/lan966x/lan966x_main.c |  5 ++
> > >  .../ethernet/microchip/lan966x/lan966x_main.h | 13 +++
> > >  .../ethernet/microchip/lan966x/lan966x_xdp.c  | 81 +++++++++++++++++++
> > >  5 files changed, 111 insertions(+), 2 deletions(-)
> > >  create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
> > 
> > [...]
> > 
> > > +bool lan966x_xdp_port_present(struct lan966x_port *port)
> > > +{
> > > +     return !!port->xdp_prog;
> > > +}
> > 
> > Why uninline such a simple check? I realize you want to keep all XDP
> > stuff inside in the separate file, but doesn't this one looks too
> > much?
> 
> I was kind of hoping that the compiler will inline it for me.
> But I can add it in the header file to inline it.

That is very unlikely for the compilers to uninline an extern
function. LTO is able to do that, but even then it's not
guaranteed. So I'd keep it in a header file as an inline.

> 
> > 
> > > +
> > > +int lan966x_xdp_port_init(struct lan966x_port *port)
> > > +{
> > > +     struct lan966x *lan966x = port->lan966x;
> > > +
> > > +     return xdp_rxq_info_reg(&port->xdp_rxq, port->dev, 0,
> > > +                             lan966x->napi.napi_id);
> > > +}
> > > +
> > > +void lan966x_xdp_port_deinit(struct lan966x_port *port)
> > > +{
> > > +     if (xdp_rxq_info_is_reg(&port->xdp_rxq))
> > > +             xdp_rxq_info_unreg(&port->xdp_rxq);
> > > +}
> > > --
> > > 2.38.0
> > 
> > Thanks,
> > Olek
> 
> -- 
> /Horatiu

Thanks,
Olek