Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

[Ilya Maximets <i.maximets@ovn.org>]

On 30.10.2019 16:49, Thomas Monjalon wrote:
> 30/10/2019 16:07, Ilya Maximets:
>> On 29.10.2019 19:50, Thomas Monjalon wrote:
>>> In a virtual environment, the network controller may have to configure
>>> some SR-IOV VF parameters for security reasons.
>>>
>>> When the PF (host port) is driven by DPDK (OVS-DPDK case),
>>> we face two different cases:
>>>       - driver is bifurcated (Mellanox case),
>>>         so the VF can be configured via the kernel.
>>>       - driver is on top of UIO or VFIO, so DPDK API is required,
>>>         and PMD-specific APIs were used.
>>
>> So, what is wrong with setting VF mac via the representor port as
>> it done now?  From our previous discussion, I thought that we concluded
>> that is enough to have current API, i.e. just call set_default_mac()
>> for a representor port and this will lead to setting mac for VF.
>> This is how it's implemented in Linux kernel and this is how it's
>> implemented in current DPDK drivers that supports setting mac for
>> the representor.
> 
> I don't know what is done in the Linux kernel.
> In DPDK, setting the MAC of the representor is really setting
> the MAC of the representor. Is it crazy?

Kind of. And no, it doesn't work this way in DPDK.
Just look at the i40e driver:

325 static int
326 i40e_vf_representor_mac_addr_set(struct rte_eth_dev *ethdev,
327         struct ether_addr *mac_addr)
328 {
329     struct i40e_vf_representor *representor = ethdev->data->dev_private;
330
331     return rte_pmd_i40e_set_vf_mac_addr(
332         representor->adapter->eth_dev->data->port_id,
333         representor->vf_id, mac_addr);
334 }
....
423 static const struct eth_dev_ops i40e_representor_dev_ops = {
     <...>
445     .mac_addr_set         = i40e_vf_representor_mac_addr_set,


It really calls the function to set VF mac address.

And for MLX drivers it's the same.
MLX driver call netlink to set representor MAC, but netlink is in
kernel and this call inside the kernel translates to the setting
of mac address of the VF.

Am I missing something?

> 
>> The only use case for this new API is to be able to control mac of
>> the representor itself, which doesn't make much sense. At least there
>> are only hypothetical use cases. And once again, Linux kernel doesn't
>> support this behavior.
> 
> I think it is sane to be able to set different MAC addresses
> for the representor and the VF.
> 
>>> This new generic API will avoid vendors fragmentation.
>>
>> I don't see the fragmentation. Right now you can set VF mac from DPDK
>> by calling set_default_mac() for representor.  This API exists for all
>> vendors. Not implemented for Intel, but new API is not implemented too.
> 
> No, the current situation is the following:
> 	- for mlx5, VF MAC can be configured with iproute2 or netlink
> 	- for ixgbe, rte_pmd_ixgbe_set_vf_mac_addr()
> 	- for i40e, rte_pmd_i40e_set_vf_mac_addr()
> 	- for bnxt, rte_pmd_bnxt_set_vf_mac_addr()
> 
>> The this is that this new API will produce conceptual fragmentation
>> between DPDK and the Linux kernel, because to do the same thing you'll
>> have to use different ways. I mean, to change mac of VF in kernel you
>> need to set mac to the representor, but in DPDK changing setting mac to
>> representor will lead to changing the mac of the representor itself,
>> not the VF. This will be really confusing for users.
> 
> I am not responsible of the choices in Linux.
> But I agree it would be interesting to check the reason of such decision.
> Rony, please could you explain?
> 
>