Re: [PATCH 1/2] usertools/rss: add driver abstractions

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, Nov 21, 2023 at 10:08 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 23 Oct 2023 10:07:11 +0200
> Robin Jarry <rjarry@redhat.com> wrote:
>
> > +class DriverInfo:
> > +    def __init__(self, key: bytes = None, reta_size: int = None):
> > +        self.__key = key
> > +        self.__reta_size = reta_size
> > +
> > +    def rss_key(self) -> bytes:
> > +        return self.__key
> > +
> > +    def reta_size(self, num_queues: int) -> int:
> > +        return self.__reta_size
> > +
> > +
> > +class MlxDriverInfo(DriverInfo):
> > +    def rss_key(self) -> bytes:
> > +        return bytes(
> > +            (
> > +                # fmt: off
> > +                # rss_hash_default_key, see drivers/net/mlx5/mlx5_rxq.c
> > +                0x2c, 0xc6, 0x81, 0xd1, 0x5b, 0xdb, 0xf4, 0xf7,
> > +                0xfc, 0xa2, 0x83, 0x19, 0xdb, 0x1a, 0x3e, 0x94,
> > +                0x6b, 0x9e, 0x38, 0xd9, 0x2c, 0x9c, 0x03, 0xd1,
> > +                0xad, 0x99, 0x44, 0xa7, 0xd9, 0x56, 0x3d, 0x59,
> > +                0x06, 0x3c, 0x25, 0xf3, 0xfc, 0x1f, 0xdc, 0x2a,
> > +                # fmt: on
> > +            )
> > +        )
> > +
> > +    def reta_size(self, num_queues: int) -> int:
> > +        if num_queues & (num_queues - 1) == 0:
> > +            # If the requested number of RX queues is power of two,
> > +            # use a table of this size.
> > +            return num_queues
> > +        # otherwise, use the maximum table size
> > +        return 512
> > +
> > +
> > +DEFAULT_DRIVERS = {
> > +    "intel": DriverInfo(
> > +        key=bytes(
> > +            (
> > +                # fmt: off
> > +                # rss_intel_key, see drivers/net/ixgbe/ixgbe_rxtx.c
> > +                0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> > +                0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> > +                0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> > +                0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> > +                0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
> > +                # fmt: on
> > +            )
> > +        ),
> > +        reta_size=128,
> > +    ),
> > +    "mlx": MlxDriverInfo(),
> > +    "i40e": DriverInfo(
> > +        key=bytes(
> > +            (
> > +                # fmt: off
> > +                # rss_key_default, see drivers/net/i40e/i40e_ethdev.c
> > +                # i40e is the only driver that takes 52 bytes keys
> > +                0x44, 0x39, 0x79, 0x6b, 0xb5, 0x4c, 0x50, 0x23,
> > +                0xb6, 0x75, 0xea, 0x5b, 0x12, 0x4f, 0x9f, 0x30,
> > +                0xb8, 0xa2, 0xc0, 0x3d, 0xdf, 0xdc, 0x4d, 0x02,
> > +                0xa0, 0x8c, 0x9b, 0x33, 0x4a, 0xf6, 0x4a, 0x4c,
> > +                0x05, 0xc6, 0xfa, 0x34, 0x39, 0x58, 0xd8, 0x55,
> > +                0x7d, 0x99, 0x58, 0x3a, 0xe1, 0x38, 0xc9, 0x2e,
> > +                0x81, 0x15, 0x03, 0x66,
> > +                # fmt: on
> > +            )
> > +        ),
> > +        reta_size=512,
> > +    ),
> >  }
>
> The tool should not need to have driver specific tables like this.
> DPDK is already riddled with enough driver specific quirks..
>
> That shows a flaw in the rss design, which should have been fixed.

Every NIC's implements standard Toeplitz hash algorithm for RSS hash generation.
Only the initial SEED is different. It is not DPDK property, all the
HW like is that as
there is no standardization on initial SEED for hash.

> I should have seen this in the earlier versions, would have rejected the patch.