Re: [dpdk-dev] [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers.

[Tom Kiely <tkiely@brocade.com>]

Sorry for the delay in replying to this thread. I was on vacation for 
the last 3 days. Please see inline for my comments.

On 12/15/2015 02:37 PM, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> Sent: Monday, December 14, 2015 9:35 PM
>> To: Ananyev, Konstantin
>> Cc: Zhang, Helin; dev@dpdk.org; Tom Kiely
>> Subject: Re: [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers.
>>
>> On Mon, 14 Dec 2015 19:57:10 +0000
>> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
>>
>>>
>>>> -----Original Message-----
>>>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>>>> Sent: Monday, December 14, 2015 7:25 PM
>>>> To: Ananyev, Konstantin
>>>> Cc: Zhang, Helin; dev@dpdk.org; Tom Kiely
>>>> Subject: Re: [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers.
>>>>
>>>> On Mon, 14 Dec 2015 19:12:26 +0000
>>>> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
>>>>
>>>>>
>>>>>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>>>>>> Sent: Friday, December 11, 2015 4:59 PM
>>>>>> To: Zhang, Helin; Ananyev, Konstantin
>>>>>> Cc: dev@dpdk.org; Tom Kiely; Stephen Hemminger
>>>>>> Subject: [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers.
>>>>>>
>>>>>> From: Tom Kiely <tkiely@brocade.com>
>>>>>>
>>>>>> SRIOV VFs support "transparent" vlans. Traffic from/to a VM
>>>>>> associated with a VF is tagged/untagged with the specified
>>>>>> vlan in a manner intended to be totally transparent to the VM.
>>>>>>
>>>>>> The vlan is specified by "ip link set <device> vf <n> vlan <v>".
>>>>>> The VM is not configured for any vlan on the VF and the VM
>>>>>> should never see these transparent vlan headers for that reason.
>>>>>>
>>>>>> However, in practice these vlan headers are being received by
>>>>>> the VM which discards the packets as that vlan is unknown to it.
>>>>>> The Linux kernel explicitly discards such vlan headers but DPDK
>>>>>> does not.
>>>>>> This patch mirrors the kernel behaviour for SRIOV VFs only
>>>>>
>>>>> I have few concerns about that approach:
>>>>>
>>>>> 1. I don't think vlan_tci info should *always* be stripped by vf RX routine.
>>>>> There could be configurations when that information might be needed by upper layer.
>>>>> Let say VF can be member of 2 or more VLANs and upper layer would like to have that information
>>>>> for further processing.
>>>>> Or special mirror VF, that does traffic snnoping, or something else.
>>>>> 2. Proposed implementation would introduce a slowdown for all VF RX routines.
>>>>> 3. From the description it seems like the aim is to clear VLAN information for the RX packet.
>>>>> Though the patch actually clears VLAN info only for the RX packet whose VLAN tag is not present inside SW copy of VFTA table.
>>>>> Which makes no much point to me:
>>>>> If VLAN is not present in HW VFTA table, then packet with that VLAN tag will be discarded by HW anyway.
>>>>> If it is present inside VFTA table (both SW & HW), then VLAN information would be preserved with and without the patch.
>>>>>
>>>>> If you need to clear VLAN information, why not to do it on the upper layer - inside your application itself?
>>>>> Either create some sort of wrapper around rx_burst(), or setup an RX call-back for your VF device.
>>>>>
>>>>> Konstantin
>>>>
>>>> The aim is to get SRIOV to work when the transparent VLAN tag feature is used.
>>>> Please talk to the Linux driver team. Similar code exists there in ixgbevf_process_skb_fields.
>>>
>>> Ah ok, I realised what you are trying to achieve now:
>>> You setup HW VFTA[] from the PF, so from VF point of view SW copy of the VFTA[] remains unset.
>>> So HW will pass VLAN packet in, but then SW will clear VLAN tag.
>>> Ok, that clears #3 above, but I think #1,2 still remain.
>> On the host, what configured is a vlan tag per VF per guest
>>
>> Tom had more info in the original mail.
>>
>> http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/28932
>>
>>>> The other option is have a copy of all the receive logic which is only
>>>> used by VF code.
>>> Why that's the only option?
>>> Why can't you clear that VLAN information above the PMD layer?
>>> Keep/obtain a copy of VFTA[] somewhere on the upper layer,
>>> and do actual clear after rx_burst() returns?
>>> Konstantin
>> The problem is that the guest is supposed to not see the VLAN tags (it has no reason to),
>> but the hardware leaves a VLAN tag on there.
> Yes, I understand what you are trying to achieve.
>   What I am trying to say:
> 1. VLAN tag removing shouldn't be forced for all VFs.
> I think there are scenarios where existing behaviour (keeping vlan_tci and ol_flags intact) are what people need.
> One example would be mirror VF doing other VFs traffic snooping.
> Probably some other cases too.
> 2. The way you implemented it - it might cause a RX performance degradation (specially for VF).
> That's why I think it better to be implemented on top of PMD:
> i.e: some sort of wrapper that checks all packets returned by rx_burst() and clears vlan_tci if needed.
> That would give you desired behaviour and keep current implementation intact.
>
> Konstantin
>
>   
>
> Hi Konstantin,
      To address your comments:

(1) Only tags corresponding to VLANs that the client knows nothing about 
are stripped. These tags are not intended to be seen by the client.
Maybe your concern would be addressed by disabling this functionality 
when snooping in the same way that vlan offloading is disabled ?
I think further analysis is required here on our part.
(2) In relation to performance, for the non-SRIOV case, the hit is one 
"if" per packet to test whether the functionality is enabled or not. We 
saw no significant performance impact for the SRIOV case.
Moving the functionality above PMD is certainly something that we can 
examine.

Thanks,

Tom
>