Re: [PATCH RFC net-next 3/4] net/mlx5: Separate between eswitch and MPFS l2 table logic

On Mon, Jun 12, 2017 at 8:52 PM, Jes Sorensen [off-list ref] wrote:

On 06/07/2017 07:42 PM, Saeed Mahameed wrote:

quoted

Multi-Physical Function Switch (MPFs) is required for when multi-PF
configuration is enabled to allow passing user configured unicast MAC
addresses to the requesting PF.

Before this patch eswitch.c used to manage the HW MPFS l2 table,
eswitch always enabled vport(0) (NIC PF) vport's contexts update on
unicast
mac address list changes, to populate the PF's MPFS L2 table accordingly,
even if SRIOV was not enabled.

In downstream patch we would like to allow compiling the driver without
eswitch functionalities, for that we move MPFS l2 table logic out
of eswitch.c into its own file, and provide Kconfig flag (MLX5_MPFS) to
allow compiling out MPFS for those who don't want Multi-PF support

NIC PF netdevice will now directly update MPFS l2 table via the new MPFS
API. VF netdevice has no access to MPFS L2 table, so e-Switch will remain
responsible of updating its PF MPFS l2 table on behalf of its VFs.

Due to this change we also don't require enabling vport(0) (PF vport)
unicast mac changes events anymore, for when SRIOV is not enabled.
Which means eswitch is now activated only on SRIOV activation, and not
required otherwise.

On overall it looks good - one nit.

quoted

+static int alloc_l2table_index(struct mlx5_mpfs *l2table, u32 *ix)
+{
+       int err = 0;
+
+       *ix = find_first_zero_bit(l2table->bitmap, l2table->size);
+       if (*ix >= l2table->size)
+               err = -ENOSPC;
+       else
+               __set_bit(*ix, l2table->bitmap);
+
+       return err;
+}

You pass in a pointer for ix but you don't modify it, why not just pass in
the value?.

we do modify ix:
   *ix = find_first_zero_bit(l2table->bitmap, l2table->size);

The idea is to find the next free index and return it to the caller

quoted

+static void free_l2table_index(struct mlx5_mpfs *l2table, u32 ix)
+{
+       __clear_bit(ix, l2table->bitmap);
+}

Here you stick to the u32.

to free the index we allocated from before

Jes