| Message ID | 20221122234825.3360663-1-jesse.brandeburg@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [net-next,v2] ixgbe: XDP: fix checker warning from rcu pointer | expand |
On 11/22/2022 3:48 PM, Jesse Brandeburg wrote: > The ixgbe driver uses an older style failure mode when initializing the > XDP program and the queues. It causes some warnings when running C=2 > checking builds (and it's the last one in the ethernet/intel tree). Um yeah, I forgot to remove the v2 from the title. This is only v1...
On Tue, Nov 22, 2022 at 03:48:25PM -0800, Jesse Brandeburg wrote: > The ixgbe driver uses an older style failure mode when initializing the > XDP program and the queues. It causes some warnings when running C=2 > checking builds (and it's the last one in the ethernet/intel tree). > > $ make W=1 C=2 M=`pwd`/drivers/net/ethernet/intel modules > .../ixgbe_main.c:10301:25: error: incompatible types in comparison expression (different address spaces): > .../ixgbe_main.c:10301:25: struct bpf_prog [noderef] __rcu * > .../ixgbe_main.c:10301:25: struct bpf_prog * > > Fix the problem by removing the line that tried to re-xchg "the old_prog > pointer" if there was an error, to make this driver act like the other > drivers which return the error code without "pointer restoration." > > Also, update the "copy the pointer" logic to use WRITE_ONCE as many/all > the other drivers do, which required making a change in two separate > functions that write the xdp_prog variable in the ring. > > The code here was modeled after the code in i40e/i40e_xdp_setup(). > > NOTE: Compile-tested only. Giving samples/bpf/xdp1 a spin wouldn't hurt :P Otherwise, this LGTM, shouldn't break anything: Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > CC: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > CC: Magnus Karlsson <magnus.karlsson@intel.com> > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index ab8370c413f3..93699d2ae051 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -6647,7 +6647,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter, > rx_ring->queue_index, ixgbe_rx_napi_id(rx_ring)) < 0) > goto err; > > - rx_ring->xdp_prog = adapter->xdp_prog; > + WRITE_ONCE(rx_ring->xdp_prog, adapter->xdp_prog); > > return 0; > err: > @@ -10297,14 +10297,13 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog) > synchronize_rcu(); > err = ixgbe_setup_tc(dev, adapter->hw_tcs); > > - if (err) { > - rcu_assign_pointer(adapter->xdp_prog, old_prog); > + if (err) > return -EINVAL; > - } > } else { > - for (i = 0; i < adapter->num_rx_queues; i++) > - (void)xchg(&adapter->rx_ring[i]->xdp_prog, > - adapter->xdp_prog); > + for (i = 0; i < adapter->num_rx_queues; i++) { > + WRITE_ONCE(adapter->rx_ring[i]->xdp_prog, > + adapter->xdp_prog); > + } > } > > if (old_prog) > > base-commit: 50ae3afd446667c6fe540f41b84ff83172c38b27 > -- > 2.31.1 >
-----Original Message----- From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Fijalkowski, Maciej Sent: 23 November 2022 21:10 To: Brandeburg, Jesse <jesse.brandeburg@intel.com> Cc: intel-wired-lan@lists.osuosl.org; Karlsson, Magnus <magnus.karlsson@intel.com> Subject: Re: [Intel-wired-lan] [PATCH net-next v2] ixgbe: XDP: fix checker warning from rcu pointer On Tue, Nov 22, 2022 at 03:48:25PM -0800, Jesse Brandeburg wrote: > The ixgbe driver uses an older style failure mode when initializing > the XDP program and the queues. It causes some warnings when running > C=2 checking builds (and it's the last one in the ethernet/intel tree). > > $ make W=1 C=2 M=`pwd`/drivers/net/ethernet/intel modules > .../ixgbe_main.c:10301:25: error: incompatible types in comparison expression (different address spaces): > .../ixgbe_main.c:10301:25: struct bpf_prog [noderef] __rcu * > .../ixgbe_main.c:10301:25: struct bpf_prog * > > Fix the problem by removing the line that tried to re-xchg "the > old_prog pointer" if there was an error, to make this driver act like > the other drivers which return the error code without "pointer restoration." > > Also, update the "copy the pointer" logic to use WRITE_ONCE as > many/all the other drivers do, which required making a change in two > separate functions that write the xdp_prog variable in the ring. > > The code here was modeled after the code in i40e/i40e_xdp_setup(). > > NOTE: Compile-tested only. Giving samples/bpf/xdp1 a spin wouldn't hurt :P Otherwise, this LGTM, shouldn't break anything: Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > CC: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > CC: Magnus Karlsson <magnus.karlsson@intel.com> > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index ab8370c413f3..93699d2ae051 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -6647,7 +6647,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter, rx_ring->queue_index, ixgbe_rx_napi_id(rx_ring)) < 0) goto err; - rx_ring->xdp_prog = adapter->xdp_prog; + WRITE_ONCE(rx_ring->xdp_prog, adapter->xdp_prog); return 0; err: @@ -10297,14 +10297,13 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog) synchronize_rcu(); err = ixgbe_setup_tc(dev, adapter->hw_tcs); - if (err) { - rcu_assign_pointer(adapter->xdp_prog, old_prog); + if (err) return -EINVAL; - } } else { - for (i = 0; i < adapter->num_rx_queues; i++) - (void)xchg(&adapter->rx_ring[i]->xdp_prog, - adapter->xdp_prog); + for (i = 0; i < adapter->num_rx_queues; i++) { + WRITE_ONCE(adapter->rx_ring[i]->xdp_prog, + adapter->xdp_prog); + } } if (old_prog)
The ixgbe driver uses an older style failure mode when initializing the XDP program and the queues. It causes some warnings when running C=2 checking builds (and it's the last one in the ethernet/intel tree). $ make W=1 C=2 M=`pwd`/drivers/net/ethernet/intel modules .../ixgbe_main.c:10301:25: error: incompatible types in comparison expression (different address spaces): .../ixgbe_main.c:10301:25: struct bpf_prog [noderef] __rcu * .../ixgbe_main.c:10301:25: struct bpf_prog * Fix the problem by removing the line that tried to re-xchg "the old_prog pointer" if there was an error, to make this driver act like the other drivers which return the error code without "pointer restoration." Also, update the "copy the pointer" logic to use WRITE_ONCE as many/all the other drivers do, which required making a change in two separate functions that write the xdp_prog variable in the ring. The code here was modeled after the code in i40e/i40e_xdp_setup(). NOTE: Compile-tested only. CC: Maciej Fijalkowski <maciej.fijalkowski@intel.com> CC: Magnus Karlsson <magnus.karlsson@intel.com> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) base-commit: 50ae3afd446667c6fe540f41b84ff83172c38b27