| Message ID | 20230313203608.1680781-2-jesse.brandeburg@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice_switch fixes series | expand |
From: Brandeburg, Jesse <jesse.brandeburg@intel.com> Date: Mon, 13 Mar 2023 13:36:07 -0700 > make modules W=1 returns: > .../ice/ice_txrx_lib.c:448: warning: Function parameter or member 'first_idx' not described in 'ice_finalize_xdp_rx' > .../ice/ice_txrx.c:948: warning: Function parameter or member 'ntc' not described in 'ice_get_rx_buf' > .../ice/ice_txrx.c:1038: warning: Excess function parameter 'rx_buf' description in 'ice_construct_skb' > > Fix these warnings by adding and deleting the deviant arguments. > > Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side") > Fixes: d7956d81f150 ("ice: Pull out next_to_clean bump out of ice_put_rx_buf()") > CC: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +- > drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c > index dfd22862e926..21c1e1bb214a 100644 > --- a/drivers/net/ethernet/intel/ice/ice_txrx.c > +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c > @@ -938,6 +938,7 @@ ice_reuse_rx_page(struct ice_rx_ring *rx_ring, struct ice_rx_buf *old_buf) > * ice_get_rx_buf - Fetch Rx buffer and synchronize data for use > * @rx_ring: Rx descriptor ring to transact packets on > * @size: size of buffer to add to skb > + * @ntc: index of next to clean element > * > * This function will pull an Rx buffer from the ring and synchronize it > * for use by the CPU. > @@ -1026,7 +1027,6 @@ ice_build_skb(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp) > /** > * ice_construct_skb - Allocate skb and populate it > * @rx_ring: Rx descriptor ring to transact packets on > - * @rx_buf: Rx buffer to pull data from > * @xdp: xdp_buff pointing to the data > * > * This function allocates an skb. It then populates it with the page > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > index 7bc5aa340c7d..c8322fb6f2b3 100644 > --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > @@ -438,6 +438,7 @@ int __ice_xmit_xdp_ring(struct xdp_buff *xdp, struct ice_tx_ring *xdp_ring, > * ice_finalize_xdp_rx - Bump XDP Tx tail and/or flush redirect map > * @xdp_ring: XDP ring > * @xdp_res: Result of the receive batch > + * @first_idx: index to write from caller > * > * This function bumps XDP Tx tail and/or flush redirect map, and > * should be called when a batch of packets has been processed in the How is Rx hotpath related to "ice_switch" O_o :D Those are 2 separate fixes or at least the cover shouldn't say they belong to one topic. Thanks, Olek
On 3/14/2023 6:54 AM, Alexander Lobakin wrote: > From: Brandeburg, Jesse <jesse.brandeburg@intel.com> >> --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c >> +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c >> @@ -438,6 +438,7 @@ int __ice_xmit_xdp_ring(struct xdp_buff *xdp, struct ice_tx_ring *xdp_ring, >> * ice_finalize_xdp_rx - Bump XDP Tx tail and/or flush redirect map >> * @xdp_ring: XDP ring >> * @xdp_res: Result of the receive batch >> + * @first_idx: index to write from caller >> * >> * This function bumps XDP Tx tail and/or flush redirect map, and >> * should be called when a batch of packets has been processed in the > > How is Rx hotpath related to "ice_switch" O_o :D Those are 2 separate > fixes or at least the cover shouldn't say they belong to one topic. Thanks for the review: Quoting myself: > Fix a couple of small issues, with a small refactor to correctly handle > a possible misconfiguration of RDMA filters, and some trivial function > headers that didn't match their function. Please see the last half of the sentence, regarding fixing these trivial comment-only issues. If you still think that isn't enough I can reword or resend, but in this case I don't think there is anything wrong, do you agree after my explanation? Did I miss something? Thanks, Jesse
From: Brandeburg, Jesse <jesse.brandeburg@intel.com> Date: Tue, 14 Mar 2023 13:29:41 -0700 > On 3/14/2023 6:54 AM, Alexander Lobakin wrote: >> From: Brandeburg, Jesse <jesse.brandeburg@intel.com> [...] > Quoting myself: >> Fix a couple of small issues, with a small refactor to correctly handle >> a possible misconfiguration of RDMA filters, and some trivial function >> headers that didn't match their function. > > Please see the last half of the sentence, regarding fixing these trivial > comment-only issues. If you still think that isn't enough I can reword > or resend, but in this case I don't think there is anything wrong, do > you agree after my explanation? Did I miss something? I've read the cover letter, but its subject is "ice_switch fixes series", that's why I wrote this :D That confuses a bit. I've no problems with this spin, Tony doesn't preserve cover letter subjects anyway. > > Thanks, > Jesse > Thanks, Olek
On 3/15/2023 2:55 AM, Alexander Lobakin wrote: > From: Brandeburg, Jesse <jesse.brandeburg@intel.com> > Date: Tue, 14 Mar 2023 13:29:41 -0700 > >> On 3/14/2023 6:54 AM, Alexander Lobakin wrote: >>> From: Brandeburg, Jesse <jesse.brandeburg@intel.com> > > [...] > >> Quoting myself: >>> Fix a couple of small issues, with a small refactor to correctly handle >>> a possible misconfiguration of RDMA filters, and some trivial function >>> headers that didn't match their function. >> >> Please see the last half of the sentence, regarding fixing these trivial >> comment-only issues. If you still think that isn't enough I can reword >> or resend, but in this case I don't think there is anything wrong, do >> you agree after my explanation? Did I miss something? > > I've read the cover letter, but its subject is "ice_switch fixes > series", that's why I wrote this :D That confuses a bit. I've no > problems with this spin, Tony doesn't preserve cover letter subjects anyway. If they are sent on as I received them, I'll carry the cover letter forward. However, if I add other patches to them, it won't be preserved. I see this getting bundled with other patches, so the subject for this likely won't matter. Thanks, Tony >> Thanks, >> Jesse >> > > Thanks, > Olek > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c index dfd22862e926..21c1e1bb214a 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx.c +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c @@ -938,6 +938,7 @@ ice_reuse_rx_page(struct ice_rx_ring *rx_ring, struct ice_rx_buf *old_buf) * ice_get_rx_buf - Fetch Rx buffer and synchronize data for use * @rx_ring: Rx descriptor ring to transact packets on * @size: size of buffer to add to skb + * @ntc: index of next to clean element * * This function will pull an Rx buffer from the ring and synchronize it * for use by the CPU. @@ -1026,7 +1027,6 @@ ice_build_skb(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp) /** * ice_construct_skb - Allocate skb and populate it * @rx_ring: Rx descriptor ring to transact packets on - * @rx_buf: Rx buffer to pull data from * @xdp: xdp_buff pointing to the data * * This function allocates an skb. It then populates it with the page diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c index 7bc5aa340c7d..c8322fb6f2b3 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c @@ -438,6 +438,7 @@ int __ice_xmit_xdp_ring(struct xdp_buff *xdp, struct ice_tx_ring *xdp_ring, * ice_finalize_xdp_rx - Bump XDP Tx tail and/or flush redirect map * @xdp_ring: XDP ring * @xdp_res: Result of the receive batch + * @first_idx: index to write from caller * * This function bumps XDP Tx tail and/or flush redirect map, and * should be called when a batch of packets has been processed in the
make modules W=1 returns: .../ice/ice_txrx_lib.c:448: warning: Function parameter or member 'first_idx' not described in 'ice_finalize_xdp_rx' .../ice/ice_txrx.c:948: warning: Function parameter or member 'ntc' not described in 'ice_get_rx_buf' .../ice/ice_txrx.c:1038: warning: Excess function parameter 'rx_buf' description in 'ice_construct_skb' Fix these warnings by adding and deleting the deviant arguments. Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side") Fixes: d7956d81f150 ("ice: Pull out next_to_clean bump out of ice_put_rx_buf()") CC: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com> --- drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +- drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)