On Mon, Jun 12, 2017 at 8:52 PM, Jes Sorensen <jsoren...@fb.com> wrote:
> On 06/07/2017 07:42 PM, Saeed Mahameed wrote:
>>
>> 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.
>
>> +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
>> +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
