netdev - Re: [Intel-wired-lan] [PATCH net-next] igc: avoid kernel warning when changing RX ring parameters

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 linux-cve-announce PHC
Open Source and information security mailing list archives

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

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

Message-ID: <YeFQH0/LY9R9GWxF@calimero.vinschen.de>
Date:   Fri, 14 Jan 2022 11:27:43 +0100
From:   Corinna Vinschen <vinschen@...hat.com>
To:     Vinicius Costa Gomes <vinicius.gomes@...el.com>
Cc:     netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
        Andre Guedes <andre.guedes@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH net-next] igc: avoid kernel warning
 when changing RX ring parameters

On Jan 13 16:37, Vinicius Costa Gomes wrote:
> Corinna Vinschen <vinschen@...hat.com> writes:
> 
> > Calling ethtool changing the RX ring parameters like this:
> >
> >   $ ethtool -G eth0 rx 1024
> >
> > triggers the "Missing unregister, handled but fix driver" warning in
> > xdp_rxq_info_reg().
> >[...]
> 
> Reaching "inside" xdp_rxq to reset it doesn't feel right in this
> context, even if it's going to work fine (for now).
> 
> I think that the suggestion that Alexander gave in that other thread is
> the best approach. And thanks for noticing that igb '_set_ringparam()'
> has the same underlying problem and also needs to be fixed.

Yeah, it didn't sit overly well with me either, but I thought if it's
good for igb...

Either way, the better approach should be sth. like this in both,
ig[bc]_setup_rx_resources:

	if (xdp_rxq_info_is_reg(&rx_ring->xdp_rxq))
		xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
	res = xdp_rxq_info_reg(&rx_ring->xdp_rxq, ndev, index,
			       rx_ring->q_vector->napi.napi_id);

And while at it, wouldn't it make sense to move the xdp_rxq_info_reg
call in igc_setup_rx_resources down to be the last action, so the error
path doesn't have to call xdp_rxq_info_unreg, just like in
igb_setup_rx_resources?


Thanks,
Corinna