[dpdk-dev] supported packet types

[dpdk-dev] supported packet types

[Olivier Matz]

Hi,

The following commit introduces a function to list the supported
packet types of a device:

  http://dpdk.org/browse/dpdk/commit/?id=78a38edf66

I would like to know what does "supported" precisely mean.
Is it:

1/ - if a ptype is marked as supported, the driver MUST set
     this ptype if the packet matches the format described in rte_mbuf.h

   -> if the ptype is not recognized, the application is sure
      that the packet is not one of the supported ptype
   -> but this is difficult to take advantage of this inside an
      application that supports several different ports model
      that do not support the same packet types

2/ - if a ptype is marked as supported, the driver CAN set
     the ptype if the packet matches the format described in rte_mbuf.h

   -> if a ptype is not recognized, the application does a software
      fallback
   -> in this case, it would useless to have the get_supported_ptype()

Can you confirm if the PMDs and l3fwd (the only user) expect 1/
or 2/ ?

Can you elaborate on the advantages of having this API?

And a supplementary question: if a ptype is not marked as supported,
is it forbidden for a driver to set this ptype anyway? Because we can
imagine a hardware that can only recognize packets in some conditions
(ex: can recognize IPv4 if there is no vlan).

I think properly defining the meaning of "supported" is mandatory
to have an application beeing able to use this feature, and avoid
PMDs to behave differently because the API is unclear (like we've
already seen for other features).


Thanks,
Olivier

Re: [dpdk-dev] supported packet types

[Ananyev, Konstantin]

Hi Olivier,

 
> Hi,
> 
> The following commit introduces a function to list the supported
> packet types of a device:
> 
>   http://dpdk.org/browse/dpdk/commit/?id=78a38edf66
> 
> I would like to know what does "supported" precisely mean.
> Is it:
> 
> 1/ - if a ptype is marked as supported, the driver MUST set
>      this ptype if the packet matches the format described in rte_mbuf.h
> 
>    -> if the ptype is not recognized, the application is sure
>       that the packet is not one of the supported ptype
>    -> but this is difficult to take advantage of this inside an
>       application that supports several different ports model
>       that do not support the same packet types
> 
> 2/ - if a ptype is marked as supported, the driver CAN set
>      the ptype if the packet matches the format described in rte_mbuf.h
> 
>    -> if a ptype is not recognized, the application does a software
>       fallback
>    -> in this case, it would useless to have the get_supported_ptype()
> 
> Can you confirm if the PMDs and l3fwd (the only user) expect 1/
> or 2/ ?

1) 

> 
> Can you elaborate on the advantages of having this API?

Application can rely on information provided by PMD avoid parsing packet manually to recognise it's pytpe.

> 
> And a supplementary question: if a ptype is not marked as supported,
> is it forbidden for a driver to set this ptype anyway?

I suppose it is not forbidden, but there is no guarantee from PMD that it
would be able to recognise that ptype.

Konstantin

> Because we can
> imagine a hardware that can only recognize packets in some conditions
> (ex: can recognize IPv4 if there is no vlan).
> 
> I think properly defining the meaning of "supported" is mandatory
> to have an application beeing able to use this feature, and avoid
> PMDs to behave differently because the API is unclear (like we've
> already seen for other features).
> 
> 
> Thanks,
> Olivier

Re: [dpdk-dev] supported packet types

[Olivier Matz]

Hi Konstantin,

On 04/29/2016 06:03 PM, Ananyev, Konstantin wrote:
>> The following commit introduces a function to list the supported
>> packet types of a device:
>>
>>   http://dpdk.org/browse/dpdk/commit/?id=78a38edf66
>>
>> I would like to know what does "supported" precisely mean.
>> Is it:
>>
>> 1/ - if a ptype is marked as supported, the driver MUST set
>>      this ptype if the packet matches the format described in rte_mbuf.h
>>
>>    -> if the ptype is not recognized, the application is sure
>>       that the packet is not one of the supported ptype
>>    -> but this is difficult to take advantage of this inside an
>>       application that supports several different ports model
>>       that do not support the same packet types
>>
>> 2/ - if a ptype is marked as supported, the driver CAN set
>>      the ptype if the packet matches the format described in rte_mbuf.h
>>
>>    -> if a ptype is not recognized, the application does a software
>>       fallback
>>    -> in this case, it would useless to have the get_supported_ptype()
>>
>> Can you confirm if the PMDs and l3fwd (the only user) expect 1/
>> or 2/ ?
> 
> 1) 
> 
>>
>> Can you elaborate on the advantages of having this API?
> 
> Application can rely on information provided by PMD avoid parsing packet manually to recognise it's pytpe.
> 
>>
>> And a supplementary question: if a ptype is not marked as supported,
>> is it forbidden for a driver to set this ptype anyway?
> 
> I suppose it is not forbidden, but there is no guarantee from PMD that it
> would be able to recognise that ptype.
> 
> Konstantin
> 
>> Because we can
>> imagine a hardware that can only recognize packets in some conditions
>> (ex: can recognize IPv4 if there is no vlan).
>>
>> I think properly defining the meaning of "supported" is mandatory
>> to have an application beeing able to use this feature, and avoid
>> PMDs to behave differently because the API is unclear (like we've
>> already seen for other features).

Back on this. I've made some tests with ixgbe, and I'm afraid it
will be difficult to ensure that when a ptype is advertised, it will
always be set in the mbuf, whatever the layers below. Here are some
examples:

- double vlans

Ether(type=0x88A8)/Dot1Q(vlan=0x666)/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)
  ixgbe advertises RTE_PTYPE_ETHER in supported ptypes
  returned ptype: RTE_PTYPE_UNKNOWN
  should be: L2_ETHER
  (this works with any unknown ethtype)

- ip6 in ip6 tunnel
  ixgbe advertises RTE_PTYPE_TUNNEL_IP in supported ptypes
  Ether()/IPv6()/IPv6()/UDP()/Raw("x"*32)
  returned ptype: L2_ETHER L3_IPV6
  should be: L2_ETHER L3_IPV6 TUNNEL_IP INNER_L3_IPV6 INNER_L4_UDP

- ip options
  Ether()/IP(options=IPOption('\x83\x03\x10'))/UDP()/Raw("x"*32)
  returned ptype: RTE_PTYPE_UNKNOWN
  should be: L2_ETHER L3_IPV4_EXT L4_UDP

- ip inside ip with options
  Ether()/IP(options=IPOption('\x83\x03\x10'))/IP()/UDP()/Raw("x"*32)
  returned ptype: L2_ETHER L3_IPV4_EXT
  should be: L2_ETHER L3_IPV4_EXT TUNNEL_IP INNER_L3_IPV4 INNER_L4_UDP

I'm sure we can find more examples that do not return the expected
result, knowing that ixgbe is probably one of the most complete
driver in dpdk. I'm afraid of the behavior for other PMDs :)

That's why I think the get_supported_ptypes() function, as of today,
is useless for an application. I suggest instead to set the ptype
in an opportunistic fashion instead:
- if the driver/hw knows the ptype, set it
- else, set it to unknown

What do you think?

By the way, I'm working on a software implementation that return a
packet_type from a mbuf. I may need it for virtio offload, but before
that, I think it could be useful for debug purposes. I'll submit a
patchset for this in the coming days.

Regards,
Olivier

PS: sorry, many questions to you these days... answer when you have
 the time ;)

Re: [dpdk-dev] supported packet types

[Adrien Mazarguil]

On Thu, Jun 09, 2016 at 09:57:28AM +0200, Olivier Matz wrote:
> Hi Konstantin,
> 
> On 04/29/2016 06:03 PM, Ananyev, Konstantin wrote:
> >> The following commit introduces a function to list the supported
> >> packet types of a device:
> >>
> >>   http://dpdk.org/browse/dpdk/commit/?id=78a38edf66
> >>
> >> I would like to know what does "supported" precisely mean.
> >> Is it:
> >>
> >> 1/ - if a ptype is marked as supported, the driver MUST set
> >>      this ptype if the packet matches the format described in rte_mbuf.h
> >>
> >>    -> if the ptype is not recognized, the application is sure
> >>       that the packet is not one of the supported ptype
> >>    -> but this is difficult to take advantage of this inside an
> >>       application that supports several different ports model
> >>       that do not support the same packet types
> >>
> >> 2/ - if a ptype is marked as supported, the driver CAN set
> >>      the ptype if the packet matches the format described in rte_mbuf.h
> >>
> >>    -> if a ptype is not recognized, the application does a software
> >>       fallback
> >>    -> in this case, it would useless to have the get_supported_ptype()
> >>
> >> Can you confirm if the PMDs and l3fwd (the only user) expect 1/
> >> or 2/ ?
> > 
> > 1) 
> > 
> >>
> >> Can you elaborate on the advantages of having this API?
> > 
> > Application can rely on information provided by PMD avoid parsing packet manually to recognise it's pytpe.
> > 
> >>
> >> And a supplementary question: if a ptype is not marked as supported,
> >> is it forbidden for a driver to set this ptype anyway?
> > 
> > I suppose it is not forbidden, but there is no guarantee from PMD that it
> > would be able to recognise that ptype.
> > 
> > Konstantin
> > 
> >> Because we can
> >> imagine a hardware that can only recognize packets in some conditions
> >> (ex: can recognize IPv4 if there is no vlan).
> >>
> >> I think properly defining the meaning of "supported" is mandatory
> >> to have an application beeing able to use this feature, and avoid
> >> PMDs to behave differently because the API is unclear (like we've
> >> already seen for other features).
> 
> Back on this. I've made some tests with ixgbe, and I'm afraid it
> will be difficult to ensure that when a ptype is advertised, it will
> always be set in the mbuf, whatever the layers below. Here are some
> examples:
> 
> - double vlans
> 
> Ether(type=0x88A8)/Dot1Q(vlan=0x666)/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)
>   ixgbe advertises RTE_PTYPE_ETHER in supported ptypes
>   returned ptype: RTE_PTYPE_UNKNOWN
>   should be: L2_ETHER
>   (this works with any unknown ethtype)
> 
> - ip6 in ip6 tunnel
>   ixgbe advertises RTE_PTYPE_TUNNEL_IP in supported ptypes
>   Ether()/IPv6()/IPv6()/UDP()/Raw("x"*32)
>   returned ptype: L2_ETHER L3_IPV6
>   should be: L2_ETHER L3_IPV6 TUNNEL_IP INNER_L3_IPV6 INNER_L4_UDP
> 
> - ip options
>   Ether()/IP(options=IPOption('\x83\x03\x10'))/UDP()/Raw("x"*32)
>   returned ptype: RTE_PTYPE_UNKNOWN
>   should be: L2_ETHER L3_IPV4_EXT L4_UDP
> 
> - ip inside ip with options
>   Ether()/IP(options=IPOption('\x83\x03\x10'))/IP()/UDP()/Raw("x"*32)
>   returned ptype: L2_ETHER L3_IPV4_EXT
>   should be: L2_ETHER L3_IPV4_EXT TUNNEL_IP INNER_L3_IPV4 INNER_L4_UDP
> 
> I'm sure we can find more examples that do not return the expected
> result, knowing that ixgbe is probably one of the most complete
> driver in dpdk. I'm afraid of the behavior for other PMDs :)

AFAIK, mlx4/mlx5 have similar issues, while they have no problem identifying
Dot1Q payloads, QinQ is another matter. For supported tunnel types such as
VXLAN, non-standard UDP ports prevent them from being identified as such
unless HW is configured properly (and then packets that use the standard UDP
port are not recognized anymore).

So yes, this API identifies what can be at best reported by PMDs in the
right context assuming all conditions are met, and PMDs do not implement
software fallbacks for obvious reasons.

> That's why I think the get_supported_ptypes() function, as of today,
> is useless for an application. I suggest instead to set the ptype
> in an opportunistic fashion instead:
> - if the driver/hw knows the ptype, set it
> - else, set it to unknown
> 
> What do you think?

Makes sense to me, supported encapsulations cannot be described easily with
the ptype API, applications cannot rely on it more than to get a rough idea
of what flags may potentially be set sometimes in received mbufs, which is
fine but not very useful.

That suggestion is also in line with the recent RX checksum discussion [1],
where basically HW not reporting something about a packet does not mean it
is the opposite.

[1] http://dpdk.org/ml/archives/dev/2016-May/039920.html

> By the way, I'm working on a software implementation that return a
> packet_type from a mbuf. I may need it for virtio offload, but before
> that, I think it could be useful for debug purposes. I'll submit a
> patchset for this in the coming days.
> 
> Regards,
> Olivier
> 
> PS: sorry, many questions to you these days... answer when you have
>  the time ;)

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] supported packet types

[Ananyev, Konstantin]

Hi Olivier,
 
> Hi Konstantin,
> 
> On 04/29/2016 06:03 PM, Ananyev, Konstantin wrote:
> >> The following commit introduces a function to list the supported
> >> packet types of a device:
> >>
> >>   http://dpdk.org/browse/dpdk/commit/?id=78a38edf66
> >>
> >> I would like to know what does "supported" precisely mean.
> >> Is it:
> >>
> >> 1/ - if a ptype is marked as supported, the driver MUST set
> >>      this ptype if the packet matches the format described in rte_mbuf.h
> >>
> >>    -> if the ptype is not recognized, the application is sure
> >>       that the packet is not one of the supported ptype
> >>    -> but this is difficult to take advantage of this inside an
> >>       application that supports several different ports model
> >>       that do not support the same packet types
> >>
> >> 2/ - if a ptype is marked as supported, the driver CAN set
> >>      the ptype if the packet matches the format described in rte_mbuf.h
> >>
> >>    -> if a ptype is not recognized, the application does a software
> >>       fallback
> >>    -> in this case, it would useless to have the get_supported_ptype()
> >>
> >> Can you confirm if the PMDs and l3fwd (the only user) expect 1/
> >> or 2/ ?
> >
> > 1)
> >
> >>
> >> Can you elaborate on the advantages of having this API?
> >
> > Application can rely on information provided by PMD avoid parsing packet manually to recognise it's pytpe.
> >
> >>
> >> And a supplementary question: if a ptype is not marked as supported,
> >> is it forbidden for a driver to set this ptype anyway?
> >
> > I suppose it is not forbidden, but there is no guarantee from PMD that it
> > would be able to recognise that ptype.
> >
> > Konstantin
> >
> >> Because we can
> >> imagine a hardware that can only recognize packets in some conditions
> >> (ex: can recognize IPv4 if there is no vlan).
> >>
> >> I think properly defining the meaning of "supported" is mandatory
> >> to have an application beeing able to use this feature, and avoid
> >> PMDs to behave differently because the API is unclear (like we've
> >> already seen for other features).
> 
> Back on this. I've made some tests with ixgbe, and I'm afraid it
> will be difficult to ensure that when a ptype is advertised, it will
> always be set in the mbuf, whatever the layers below. Here are some
> examples:
> 

1)

> - double vlans
> 
> Ether(type=0x88A8)/Dot1Q(vlan=0x666)/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)
>   ixgbe advertises RTE_PTYPE_ETHER in supported ptypes
>   returned ptype: RTE_PTYPE_UNKNOWN
>   should be: L2_ETHER
>   (this works with any unknown ethtype)

2)

> 
> - ip6 in ip6 tunnel
>   ixgbe advertises RTE_PTYPE_TUNNEL_IP in supported ptypes
>   Ether()/IPv6()/IPv6()/UDP()/Raw("x"*32)
>   returned ptype: L2_ETHER L3_IPV6
>   should be: L2_ETHER L3_IPV6 TUNNEL_IP INNER_L3_IPV6 INNER_L4_UDP

3)

> 
> - ip options
>   Ether()/IP(options=IPOption('\x83\x03\x10'))/UDP()/Raw("x"*32)
>   returned ptype: RTE_PTYPE_UNKNOWN
>   should be: L2_ETHER L3_IPV4_EXT L4_UDP

4)

> 
> - ip inside ip with options
>   Ether()/IP(options=IPOption('\x83\x03\x10'))/IP()/UDP()/Raw("x"*32)
>   returned ptype: L2_ETHER L3_IPV4_EXT
>   should be: L2_ETHER L3_IPV4_EXT TUNNEL_IP INNER_L3_IPV4 INNER_L4_UDP


I am marked your test-cases with numbers to simplify referencing to them.
1) & 3) - I believe the reason is a bug in our ptype code inside ixgbe PMD :(
I submitted a patch:
http://dpdk.org/dev/patchwork/patch/13820/
Could you give it a try?
I think it should fix these issues.

2) & 4) - unfortunately ixgbe HW supports only ipv4->ipv6 tunneling.
All other combinations are not supported.
Right now I didn't decide what is a best way to address this problem.
Two thoughts  I have right now:
- either  remove tunnelling recognition (RTE_PTYPE_TUNNEL_IP and RTE_PTYPE_INNER_*)
from supported ptypes in ixgbe_dev_supported_ptypes_get().
- or split RTE_PTYPE_TUNNEL_IP into RTE_PTYPE_TUNNEL_IPV4 and RTE_PTYPE_TUNNEL_IPV6.
But that looks a bit ugly, again wuld probably be required for VXLAN/GRE and might be other
tunnelling protocols in future.
So don't really like any of them.
If you have any better idea, please shout.

> 
> I'm sure we can find more examples that do not return the expected
> result, knowing that ixgbe is probably one of the most complete
> driver in dpdk. I'm afraid of the behavior for other PMDs :)

It is in general, but not for that particular feature :(

> 
> That's why I think the get_supported_ptypes() function, as of today,
> is useless for an application.

Hmm for me right now it looks more like incorrect implementation.

> I suggest instead to set the ptype
> in an opportunistic fashion instead:
> - if the driver/hw knows the ptype, set it
> - else, set it to unknown

That's what PMD does now... and I don't think it can do much more -
only interpret information provided by the HW in a proper way.
Probably I misunderstood you here...

> 
> What do you think?
> 
> By the way, I'm working on a software implementation that return a
> packet_type from a mbuf. I may need it for virtio offload, but before
> that, I think it could be useful for debug purposes. I'll submit a
> patchset for this in the coming days.

Would be interesting to see.
I had to develop something similar - so my one is not totally generic,
as that app is only interested in L3/L4 types (no tunnelling, etc.). 


> 
> Regards,
> Olivier
> 
> PS: sorry, many questions to you these days... answer when you have
>  the time ;)

Thanks for understanding and sorry for delay :)
Konstantin

Re: [dpdk-dev] supported packet types

[Olivier MATZ]

Hi Konstantin,

On 06/15/2016 04:08 PM, Ananyev, Konstantin wrote:
>
> Hi Olivier,
>
>> Hi Konstantin,
>>
>> On 04/29/2016 06:03 PM, Ananyev, Konstantin wrote:
>>>> The following commit introduces a function to list the supported
>>>> packet types of a device:
>>>>
>>>>    http://dpdk.org/browse/dpdk/commit/?id=78a38edf66
>>>>
>>>> I would like to know what does "supported" precisely mean.
>>>> Is it:
>>>>
>>>> 1/ - if a ptype is marked as supported, the driver MUST set
>>>>       this ptype if the packet matches the format described in rte_mbuf.h
>>>>
>>>>     -> if the ptype is not recognized, the application is sure
>>>>        that the packet is not one of the supported ptype
>>>>     -> but this is difficult to take advantage of this inside an
>>>>        application that supports several different ports model
>>>>        that do not support the same packet types
>>>>
>>>> 2/ - if a ptype is marked as supported, the driver CAN set
>>>>       the ptype if the packet matches the format described in rte_mbuf.h
>>>>
>>>>     -> if a ptype is not recognized, the application does a software
>>>>        fallback
>>>>     -> in this case, it would useless to have the get_supported_ptype()
>>>>
>>>> Can you confirm if the PMDs and l3fwd (the only user) expect 1/
>>>> or 2/ ?
>>>
>>> 1)
>>>
>>>>
>>>> Can you elaborate on the advantages of having this API?
>>>
>>> Application can rely on information provided by PMD avoid parsing packet manually to recognise it's pytpe.
>>>
>>>>
>>>> And a supplementary question: if a ptype is not marked as supported,
>>>> is it forbidden for a driver to set this ptype anyway?
>>>
>>> I suppose it is not forbidden, but there is no guarantee from PMD that it
>>> would be able to recognise that ptype.
>>>
>>> Konstantin
>>>
>>>> Because we can
>>>> imagine a hardware that can only recognize packets in some conditions
>>>> (ex: can recognize IPv4 if there is no vlan).
>>>>
>>>> I think properly defining the meaning of "supported" is mandatory
>>>> to have an application beeing able to use this feature, and avoid
>>>> PMDs to behave differently because the API is unclear (like we've
>>>> already seen for other features).
>>
>> Back on this. I've made some tests with ixgbe, and I'm afraid it
>> will be difficult to ensure that when a ptype is advertised, it will
>> always be set in the mbuf, whatever the layers below. Here are some
>> examples:
>>
>
> 1)
>
>> - double vlans
>>
>> Ether(type=0x88A8)/Dot1Q(vlan=0x666)/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)
>>    ixgbe advertises RTE_PTYPE_ETHER in supported ptypes
>>    returned ptype: RTE_PTYPE_UNKNOWN
>>    should be: L2_ETHER
>>    (this works with any unknown ethtype)
>
> 2)
>
>>
>> - ip6 in ip6 tunnel
>>    ixgbe advertises RTE_PTYPE_TUNNEL_IP in supported ptypes
>>    Ether()/IPv6()/IPv6()/UDP()/Raw("x"*32)
>>    returned ptype: L2_ETHER L3_IPV6
>>    should be: L2_ETHER L3_IPV6 TUNNEL_IP INNER_L3_IPV6 INNER_L4_UDP
>
> 3)
>
>>
>> - ip options
>>    Ether()/IP(options=IPOption('\x83\x03\x10'))/UDP()/Raw("x"*32)
>>    returned ptype: RTE_PTYPE_UNKNOWN
>>    should be: L2_ETHER L3_IPV4_EXT L4_UDP
>
> 4)
>
>>
>> - ip inside ip with options
>>    Ether()/IP(options=IPOption('\x83\x03\x10'))/IP()/UDP()/Raw("x"*32)
>>    returned ptype: L2_ETHER L3_IPV4_EXT
>>    should be: L2_ETHER L3_IPV4_EXT TUNNEL_IP INNER_L3_IPV4 INNER_L4_UDP
>
>
> I am marked your test-cases with numbers to simplify referencing to them.
> 1) & 3) - I believe the reason is a bug in our ptype code inside ixgbe PMD :(
> I submitted a patch:
> http://dpdk.org/dev/patchwork/patch/13820/
> Could you give it a try?
> I think it should fix these issues.

1) works, I now have L2_ETHER
3) works, I now have L2_ETHER L3_IPV4_EXT L4_UDP

I'll answer to the other thread to so it appears on patchwork.

>
> 2) & 4) - unfortunately ixgbe HW supports only ipv4->ipv6 tunneling.
> All other combinations are not supported.
> Right now I didn't decide what is a best way to address this problem.
> Two thoughts  I have right now:
> - either  remove tunnelling recognition (RTE_PTYPE_TUNNEL_IP and RTE_PTYPE_INNER_*)
> from supported ptypes in ixgbe_dev_supported_ptypes_get().
> - or split RTE_PTYPE_TUNNEL_IP into RTE_PTYPE_TUNNEL_IPV4 and RTE_PTYPE_TUNNEL_IPV6.
> But that looks a bit ugly, again wuld probably be required for VXLAN/GRE and might be other
> tunnelling protocols in future.
> So don't really like any of them.
> If you have any better idea, please shout.

No better idea. I'd say the first one is better because it does not
require to change the ptypes definition.

Another idea (but probably worst) is to do a software fallback
in the PMD, but I don't believe the PMD is the proper place for that.

>> I'm sure we can find more examples that do not return the expected
>> result, knowing that ixgbe is probably one of the most complete
>> driver in dpdk. I'm afraid of the behavior for other PMDs :)
>
> It is in general, but not for that particular feature :(
>
>>
>> That's why I think the get_supported_ptypes() function, as of today,
>> is useless for an application.
>
> Hmm for me right now it looks more like incorrect implementation.

OK

Do you mind if I send a patch to precise a bit what
supported ptype should return? I mean highlighting the fact
that it should return the ptypes that are _always_ recognized,
not the ptypes that _can_ be recognized. But it does not prevent
a PMD to set a ptype that is not in the supported list when
it knows it is correct.

>> I suggest instead to set the ptype
>> in an opportunistic fashion instead:
>> - if the driver/hw knows the ptype, set it
>> - else, set it to unknown
>
> That's what PMD does now... and I don't think it can do much more -
> only interpret information provided by the HW in a proper way.
> Probably I misunderstood you here...

My suggestion was to remove get_supported_ptypes an set the ptype in
mbuf when the pmd recognize a type.

But we could also keep get_supported_ptypes() for ptypes that will
always be recognized, allowing a PMD to set any other ptype if it
is recognized. This is probably what we have today in some PMDs, I
think it just requires some clarification.

>
>>
>> What do you think?
>>
>> By the way, I'm working on a software implementation that return a
>> packet_type from a mbuf. I may need it for virtio offload, but before
>> that, I think it could be useful for debug purposes. I'll submit a
>> patchset for this in the coming days.
>
> Would be interesting to see.
> I had to develop something similar - so my one is not totally generic,
> as that app is only interested in L3/L4 types (no tunnelling, etc.).

I'll send it soon to the ML.


Olivier

Re: [dpdk-dev] supported packet types

[Ananyev, Konstantin]

Hi Olivier,

> 
> Hi Konstantin,
> 
> On 06/15/2016 04:08 PM, Ananyev, Konstantin wrote:
> >
> > Hi Olivier,
> >
> >> Hi Konstantin,
> >>
> >> On 04/29/2016 06:03 PM, Ananyev, Konstantin wrote:
> >>>> The following commit introduces a function to list the supported
> >>>> packet types of a device:
> >>>>
> >>>>    http://dpdk.org/browse/dpdk/commit/?id=78a38edf66
> >>>>
> >>>> I would like to know what does "supported" precisely mean.
> >>>> Is it:
> >>>>
> >>>> 1/ - if a ptype is marked as supported, the driver MUST set
> >>>>       this ptype if the packet matches the format described in rte_mbuf.h
> >>>>
> >>>>     -> if the ptype is not recognized, the application is sure
> >>>>        that the packet is not one of the supported ptype
> >>>>     -> but this is difficult to take advantage of this inside an
> >>>>        application that supports several different ports model
> >>>>        that do not support the same packet types
> >>>>
> >>>> 2/ - if a ptype is marked as supported, the driver CAN set
> >>>>       the ptype if the packet matches the format described in rte_mbuf.h
> >>>>
> >>>>     -> if a ptype is not recognized, the application does a software
> >>>>        fallback
> >>>>     -> in this case, it would useless to have the get_supported_ptype()
> >>>>
> >>>> Can you confirm if the PMDs and l3fwd (the only user) expect 1/
> >>>> or 2/ ?
> >>>
> >>> 1)
> >>>
> >>>>
> >>>> Can you elaborate on the advantages of having this API?
> >>>
> >>> Application can rely on information provided by PMD avoid parsing packet manually to recognise it's pytpe.
> >>>
> >>>>
> >>>> And a supplementary question: if a ptype is not marked as supported,
> >>>> is it forbidden for a driver to set this ptype anyway?
> >>>
> >>> I suppose it is not forbidden, but there is no guarantee from PMD that it
> >>> would be able to recognise that ptype.
> >>>
> >>> Konstantin
> >>>
> >>>> Because we can
> >>>> imagine a hardware that can only recognize packets in some conditions
> >>>> (ex: can recognize IPv4 if there is no vlan).
> >>>>
> >>>> I think properly defining the meaning of "supported" is mandatory
> >>>> to have an application beeing able to use this feature, and avoid
> >>>> PMDs to behave differently because the API is unclear (like we've
> >>>> already seen for other features).
> >>
> >> Back on this. I've made some tests with ixgbe, and I'm afraid it
> >> will be difficult to ensure that when a ptype is advertised, it will
> >> always be set in the mbuf, whatever the layers below. Here are some
> >> examples:
> >>
> >
> > 1)
> >
> >> - double vlans
> >>
> >> Ether(type=0x88A8)/Dot1Q(vlan=0x666)/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)
> >>    ixgbe advertises RTE_PTYPE_ETHER in supported ptypes
> >>    returned ptype: RTE_PTYPE_UNKNOWN
> >>    should be: L2_ETHER
> >>    (this works with any unknown ethtype)
> >
> > 2)
> >
> >>
> >> - ip6 in ip6 tunnel
> >>    ixgbe advertises RTE_PTYPE_TUNNEL_IP in supported ptypes
> >>    Ether()/IPv6()/IPv6()/UDP()/Raw("x"*32)
> >>    returned ptype: L2_ETHER L3_IPV6
> >>    should be: L2_ETHER L3_IPV6 TUNNEL_IP INNER_L3_IPV6 INNER_L4_UDP
> >
> > 3)
> >
> >>
> >> - ip options
> >>    Ether()/IP(options=IPOption('\x83\x03\x10'))/UDP()/Raw("x"*32)
> >>    returned ptype: RTE_PTYPE_UNKNOWN
> >>    should be: L2_ETHER L3_IPV4_EXT L4_UDP
> >
> > 4)
> >
> >>
> >> - ip inside ip with options
> >>    Ether()/IP(options=IPOption('\x83\x03\x10'))/IP()/UDP()/Raw("x"*32)
> >>    returned ptype: L2_ETHER L3_IPV4_EXT
> >>    should be: L2_ETHER L3_IPV4_EXT TUNNEL_IP INNER_L3_IPV4 INNER_L4_UDP
> >
> >
> > I am marked your test-cases with numbers to simplify referencing to them.
> > 1) & 3) - I believe the reason is a bug in our ptype code inside ixgbe PMD :(
> > I submitted a patch:
> > http://dpdk.org/dev/patchwork/patch/13820/
> > Could you give it a try?
> > I think it should fix these issues.
> 
> 1) works, I now have L2_ETHER
> 3) works, I now have L2_ETHER L3_IPV4_EXT L4_UDP

Great, thanks for trying :)

> 
> I'll answer to the other thread to so it appears on patchwork.
> 
> >
> > 2) & 4) - unfortunately ixgbe HW supports only ipv4->ipv6 tunneling.
> > All other combinations are not supported.
> > Right now I didn't decide what is a best way to address this problem.
> > Two thoughts  I have right now:
> > - either  remove tunnelling recognition (RTE_PTYPE_TUNNEL_IP and RTE_PTYPE_INNER_*)
> > from supported ptypes in ixgbe_dev_supported_ptypes_get().
> > - or split RTE_PTYPE_TUNNEL_IP into RTE_PTYPE_TUNNEL_IPV4 and RTE_PTYPE_TUNNEL_IPV6.
> > But that looks a bit ugly, again wuld probably be required for VXLAN/GRE and might be other
> > tunnelling protocols in future.
> > So don't really like any of them.
> > If you have any better idea, please shout.
> 
> No better idea. I'd say the first one is better because it does not
> require to change the ptypes definition.

Agree.

> 
> Another idea (but probably worst) is to do a software fallback
> in the PMD, but I don't believe the PMD is the proper place for that.

Neither do I.

> 
> >> I'm sure we can find more examples that do not return the expected
> >> result, knowing that ixgbe is probably one of the most complete
> >> driver in dpdk. I'm afraid of the behavior for other PMDs :)
> >
> > It is in general, but not for that particular feature :(
> >
> >>
> >> That's why I think the get_supported_ptypes() function, as of today,
> >> is useless for an application.
> >
> > Hmm for me right now it looks more like incorrect implementation.
> 
> OK
> 
> Do you mind if I send a patch to precise a bit what
> supported ptype should return? I mean highlighting the fact
> that it should return the ptypes that are _always_ recognized,
> not the ptypes that _can_ be recognized. But it does not prevent
> a PMD to set a ptype that is not in the supported list when
> it knows it is correct.

Yes, I think it would be a good thing.

> 
> >> I suggest instead to set the ptype
> >> in an opportunistic fashion instead:
> >> - if the driver/hw knows the ptype, set it
> >> - else, set it to unknown
> >
> > That's what PMD does now... and I don't think it can do much more -
> > only interpret information provided by the HW in a proper way.
> > Probably I misunderstood you here...
> 
> My suggestion was to remove get_supported_ptypes an set the ptype in
> mbuf when the pmd recognize a type.
> 
> But we could also keep get_supported_ptypes() for ptypes that will
> always be recognized, allowing a PMD to set any other ptype if it
> is recognized. This is probably what we have today in some PMDs, I
> think it just requires some clarification.

Yes, +1 to the second option.

Konstantin

Re: [dpdk-dev] supported packet types

[Olivier Matz]

Hi Konstantin,

On 06/16/2016 01:29 PM, Ananyev, Konstantin wrote:
>>>> I suggest instead to set the ptype
>>>> in an opportunistic fashion instead:
>>>> - if the driver/hw knows the ptype, set it
>>>> - else, set it to unknown
>>>
>>> That's what PMD does now... and I don't think it can do much more -
>>> only interpret information provided by the HW in a proper way.
>>> Probably I misunderstood you here...
>>
>> My suggestion was to remove get_supported_ptypes an set the ptype in
>> mbuf when the pmd recognize a type.
>>
>> But we could also keep get_supported_ptypes() for ptypes that will
>> always be recognized, allowing a PMD to set any other ptype if it
>> is recognized. This is probably what we have today in some PMDs, I
>> think it just requires some clarification.
> 
> Yes, +1 to the second option.

What about the following API comment?

'''
Retrieve the supported packet types of an Ethernet device.

When a packet type is announced as supported, it *must* be recognized by
the PMD. For instance, if RTE_PTYPE_L2_ETHER, RTE_PTYPE_L2_ETHER_VLAN
and RTE_PTYPE_L3_IPV4 are announced, the PMD must return the following
packet types for these packets:
- Ether/IPv4              -> RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4
- Ether/Vlan/IPv4         -> RTE_PTYPE_L2_ETHER_VLAN | RTE_PTYPE_L3_IPV4
- Ether/<anything else>   -> RTE_PTYPE_L2_ETHER
- Ether/Vlan/<anything else> -> RTE_PTYPE_L2_ETHER_VLAN

When a packet is received by a PMD, the most precise type must be
returned among the ones supported. However a PMD is allowed to set
packet type that is not in the supported list, at the condition that it
is more precise. Therefore, a PMD announcing no supported packet types
can still set a matching packet type in a received packet.
'''

If it's fine I'll submit it as a patch.

Regards,
Olivier

Re: [dpdk-dev] supported packet types

[Ananyev, Konstantin]

Hi Olivier,

> 
> Hi Konstantin,
> 
> On 06/16/2016 01:29 PM, Ananyev, Konstantin wrote:
> >>>> I suggest instead to set the ptype
> >>>> in an opportunistic fashion instead:
> >>>> - if the driver/hw knows the ptype, set it
> >>>> - else, set it to unknown
> >>>
> >>> That's what PMD does now... and I don't think it can do much more -
> >>> only interpret information provided by the HW in a proper way.
> >>> Probably I misunderstood you here...
> >>
> >> My suggestion was to remove get_supported_ptypes an set the ptype in
> >> mbuf when the pmd recognize a type.
> >>
> >> But we could also keep get_supported_ptypes() for ptypes that will
> >> always be recognized, allowing a PMD to set any other ptype if it
> >> is recognized. This is probably what we have today in some PMDs, I
> >> think it just requires some clarification.
> >
> > Yes, +1 to the second option.
> 
> What about the following API comment?
> 
> '''
> Retrieve the supported packet types of an Ethernet device.
> 
> When a packet type is announced as supported, it *must* be recognized by
> the PMD. For instance, if RTE_PTYPE_L2_ETHER, RTE_PTYPE_L2_ETHER_VLAN
> and RTE_PTYPE_L3_IPV4 are announced, the PMD must return the following
> packet types for these packets:
> - Ether/IPv4              -> RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4
> - Ether/Vlan/IPv4         -> RTE_PTYPE_L2_ETHER_VLAN | RTE_PTYPE_L3_IPV4
> - Ether/<anything else>   -> RTE_PTYPE_L2_ETHER
> - Ether/Vlan/<anything else> -> RTE_PTYPE_L2_ETHER_VLAN
> 
> When a packet is received by a PMD, the most precise type must be
> returned among the ones supported. However a PMD is allowed to set
> packet type that is not in the supported list, at the condition that it
> is more precise. Therefore, a PMD announcing no supported packet types
> can still set a matching packet type in a received packet.
> '''
> 
> If it's fine I'll submit it as a patch.

Yep, looks good to me.
Thanks
Konstantin