netdev - Re: [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Jan 2022 10:31:13 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     sundeep subbaraya <sundeep.lkml@...il.com>
Cc:     Subbaraya Sundeep <sbhatta@...vell.com>,
        David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        hariprasad <hkelam@...vell.com>,
        Geetha sowjanya <gakula@...vell.com>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>
Subject: Re: [net-next PATCH] octeontx2-pf: Change receive buffer size using
 ethtool

On Thu, 13 Jan 2022 12:07:42 +0530 sundeep subbaraya wrote:
> > Should we use rx_buf_len = 0 as a way for users to reset the rxbuf len
> > to the default value? I think that would be handy.
> >  
> Before this patch we calculate each receive buffer based on mtu set by user.
> Now user can use rx-buf-len but the old mtu based calculation is also there.
> Here I am using rx_buf_len == 0 as a switch to calculate buffer length
> using mtu or
> just use length set by user. So here I am not setting rx_buf_len to some
> default value.
> 
> > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > > @@ -66,6 +66,8 @@ static int otx2_change_mtu(struct net_device *netdev, int new_mtu)
> > >                   netdev->mtu, new_mtu);
> > >       netdev->mtu = new_mtu;
> > >
> > > +     pf->hw.rbuf_len = 0;  
> >
> > Why reset the buf len on mtu change?
> >  
> As explained above buffer size will be calculated using mtu
> now instead of rx-buf-len from ethtool.

IIUC you're saying that the way to get back to the default buffer size
is to change the MTU?

It was discussed on the list in the past in the context of RSS
indirection tables and the conclusion was that we should not reset
explicit user configuration (in case of RSS indir table this means
user-set table survives change in the number of queues).

Having one config reset another is pretty painful to deal with for 
the users. I had to fix many cases of this sort of problem in the
production env I'm working with. Turning one knob resets other knobs 
so it takes multiple runs of the config daemon (chef or alike) to
get to the target configuration.

In my mind if user has set the rx-buf-len it should survive all other
config changes. User can reset to default by setting rx-buf-len to 0.
It should be possible to set rx-buf-len and mtu in any order and have
the same result.