[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <71aa9016c087e4c8d502d835ef2cddad42b56fc1.camel@kernel.org>
Date: Mon, 07 Dec 2020 15:20:03 -0800
From: Saeed Mahameed <saeed@...nel.org>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Alexander Duyck <alexander.duyck@...il.com>
Cc: Lorenzo Bianconi <lorenzo@...nel.org>, bpf <bpf@...r.kernel.org>,
Netdev <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, shayagr@...zon.com,
"Jubran, Samih" <sameehj@...zon.com>,
John Fastabend <john.fastabend@...il.com>, dsahern@...nel.org,
Jesper Dangaard Brouer <brouer@...hat.com>,
Eelco Chaudron <echaudro@...hat.com>,
lorenzo.bianconi@...hat.com, Jason Wang <jasowang@...hat.com>
Subject: Re: [PATCH v5 bpf-next 02/14] xdp: initialize xdp_buff mb bit to 0
in all XDP drivers
On Mon, 2020-12-07 at 22:37 +0100, Maciej Fijalkowski wrote:
> On Mon, Dec 07, 2020 at 01:15:00PM -0800, Alexander Duyck wrote:
> > On Mon, Dec 7, 2020 at 8:36 AM Lorenzo Bianconi <lorenzo@...nel.org
> > > wrote:
> > > Initialize multi-buffer bit (mb) to 0 in all XDP-capable drivers.
> > > This is a preliminary patch to enable xdp multi-buffer support.
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> >
> > I'm really not a fan of this design. Having to update every driver
> > in
> > order to initialize a field that was fragmented is a pain. At a
> > minimum it seems like it might be time to consider introducing some
> > sort of initializer function for this so that you can update things
> > in
> > one central place the next time you have to add a new field instead
> > of
> > having to update every individual driver that supports XDP.
> > Otherwise
> > this isn't going to scale going forward.
>
> Also, a good example of why this might be bothering for us is a fact
> that
> in the meantime the dpaa driver got XDP support and this patch hasn't
> been
> updated to include mb setting in that driver.
>
something like
init_xdp_buff(hard_start, headroom, len, frame_sz, rxq);
would work for most of the drivers.
Powered by blists - more mailing lists