Subject: Re: [net-next 02/10] udp: Expand UDP tunnel common APIs

From: Alexander Duyck <[email-masked]>
Date: Tue, 22 Jul 2014 19:16:54 -0700

On 07/22/2014 04:24 PM, Tom Herbert wrote:
> On Tue, Jul 22, 2014 at 3:55 PM, Alexander Duyck
> <[email-masked]> wrote:
>> On 07/22/2014 03:38 PM, Tom Herbert wrote:
>>> On Tue, Jul 22, 2014 at 2:56 PM, Jesse Gross <[email-masked]> wrote:
>>>> On Tue, Jul 22, 2014 at 5:16 PM, Tom Herbert <[email-masked]> wrote:
>>>>> On Tue, Jul 22, 2014 at 2:02 PM, Andy Zhou <[email-masked]> wrote:
>>>>>> On Tue, Jul 22, 2014 at 12:52 PM, Tom Herbert <[email-masked]> wrote:
>>>>>>>> --- a/include/net/udp_tunnel.h
>>>>>>>> +++ b/include/net/udp_tunnel.h
>>>>>>>> @@ -1,7 +1,10 @@
>>>>>>>>  #ifndef __NET_UDP_TUNNEL_H
>>>>>>>>  #define __NET_UDP_TUNNEL_H
>>>>>>>>
>>>>>>>> -#define UDP_TUNNEL_TYPE_VXLAN 0x01
>>>>>>>> +#include <net/ip_tunnels.h>
>>>>>>>> +
>>>>>>>> +#define UDP_TUNNEL_TYPE_VXLAN  0x01
>>>>>>>> +#define UDP_TUNNEL_TYPE_GENEVE 0x02
>>>>>>>>
>>>>>>> Why do we need to define these? Caller should know what type of port is
>>>>>>> being opened and provide appropriate encap_rcv.
>>>>>>
>>>>>> Assume udp tunnel layer needs to keep track of open ports, should it
>>>>>> also keep track of the protocol associated with the port?
>>>>>>
>>>>> For what purpose? Other than for offloads and rcv_encap functions that
>>>>> provide the service function anyway, what need is there for UDP layer
>>>>> to know about this. More to the point, if I add a module to the kernel
>>>>> with a new flavor of UDP tunneling, I shouldn't have to touch any core
>>>>> code for things to work correctly. So by this line of thinking,
>>>>> neither the terms VXLAN nor GENEVE should appear in any common code.
>>>>
>>>> The hardware will need to know what the header format is so that it
>>>> can parse the packets on receive. And since the NIC can't exactly call
>>>> into a function pointer like GRO can, I'm not sure that there is a
>>>> solution that doesn't involve an identifier that needs to be listed
>>>> somewhere. This is a pretty minimal impact - it doesn't actually
>>>> appear in the core code.
>>>
>>> The hardware doesn't *need* to know this, it's must be optional and
>>> should have no bearing on the software stack. Suggest to put them in
>>> their own header file. Also, as HW features these should appear in
>>> NETIF_F_* list so that we can control on a per device level rather to
>>> enable this feature (something like how NETIF_F_GSO_* was done).
>>>
>>> What about support for L2TP/UDP?
>>
>> The hardware needs some means of knowing what UDP port numbers are used
>> for VXLAN and/or GENEVE as the two formats contain subtle differences
>> that we have to be ready for on the Rx path as we have to parse out the
>> frames.
>>
>> We already have feature flags controlling the offloads, what the port
>> numbers provide is a means for us to determine what Rx packets we should
>> parse as tunnels vs standard UDP and which tunnel type we should parse
>> it as.
>>
> Which feature flags control the receive side parsing in the device?

The only real features that need the port info are Rx hash and Rx
checksum.  If those are disabled then there shouldn't be any need for
the port numbers.  I don't recall if you can disable them separately
from the non-tunnel case though.  I believe they are linked to the
standard offloads.

Thanks,

Alex