> From: Bruce Richardson [mailto:bruce.richard...@intel.com]
> Sent: Wednesday, 4 June 2025 11.32
>
> On Fri, May 30, 2025 at 02:57:15PM +0100, Anatoly Burakov wrote:
> > There is certain amount of duplication between various drivers when
> it
> > comes to Rx ring rearm. This patch takes implementation from ice
> driver
> > as a base because it has support for no IOVA in mbuf as well as all
> > vector implementations, and moves them to a common file.
> >
> > While we're at it, also make sure to use common definitions for
> things like
> > burst size, rearm threshold, and descriptors per loop, which is
> currently
> > defined separately in each driver.
> >
> > Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
> > ---
> >
>
> Acked-by: Bruce Richardson <bruce.richard...@intel.com>
>
> One minor comment inline below.
>
[...]
> > +
> > +static __rte_always_inline void
> > +ci_rxq_rearm(struct ci_rx_queue *rxq, const enum ci_rx_vec_level
> vec_level)
>
> I think a comment on this function would be good to point out that
> since
> it's inlined from the header and the final parameter is a compile-time
> constant, the compiler eliminates all the branches in the switch
> statement
> below.
>
> > +{
> > + const uint16_t rearm_thresh = CI_VPMD_RX_REARM_THRESH;
> > + uint16_t rx_id;
> > +
> > + /* Pull 'n' more MBUFs into the software ring */
> > + if (_ci_rxq_rearm_get_bufs(rxq) < 0)
> > + return;
> > +
> > +#ifdef RTE_NET_INTEL_USE_16BYTE_DESC
> > + switch (vec_level) {
> > + case CI_RX_VEC_LEVEL_AVX512:
> > +#ifdef __AVX512VL__
> > + _ci_rxq_rearm_avx512(rxq);
> > + break;
> > +#else
> > + /* fall back to AVX2 */
> > + /* fall through */
> > +#endif
> > + case CI_RX_VEC_LEVEL_AVX2:
> > +#ifdef __AVX2__
> > + _ci_rxq_rearm_avx2(rxq);
> > + break;
> > +#else
> > + /* fall back to SSE */
> > + /* fall through */
> > +#endif
> > + case CI_RX_VEC_LEVEL_SSE:
> > + _ci_rxq_rearm_sse(rxq, desc_len);
> > + break;
> > + }
> > +#else
> > + /* for 32-byte descriptors only support SSE */
> > + switch (vec_level) {
> > + case CI_RX_VEC_LEVEL_AVX512:
If you are respinning this patch, add "/* fall through */" here.
> > + case CI_RX_VEC_LEVEL_AVX2:
And here.
> > + case CI_RX_VEC_LEVEL_SSE:
> > + _ci_rxq_rearm_sse(rxq);
> > + break;
> > + }
> > +#endif /* RTE_NET_INTEL_USE_16BYTE_DESC */
> <snip>
Please try building with 32-byte descriptors; the compiler should have
complained about the implicit fall through.
