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
