-----Original Message-----
From: Pravin Shelar [mailto:pshe...@nicira.com] 
Sent: Thursday, August 14, 2014 8:37 AM
To: Wenyu Zhang
Cc: Ben Pfaff; Jesse Gross; Romain Lenglet; dev@openvswitch.org
Subject: Re: [PATCH v8] Extend OVS IPFIX exporter to export tunnel headers

On Wed, Aug 13, 2014 at 5:25 PM, Wenyu Zhang <wen...@vmware.com> wrote:
>>> Signed-off-by: Wenyu Zhang <wen...@vmware.com>
>>> Acked-by: Romain Lenglet <rleng...@vmware.com>
>>
>> This patch renumbers OVS_TUNNEL_KEY_ATTR_OAM and 
>> OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS.  Is that really OK?
>>
> It is not ok.
> There are another fixes to kernel part which I was going to do before pushing 
> this patch to master.
>
> Thanks,
> Pravin.
>
> Wenyu: I can move the new items after OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS if it 
> is not OK.
> And I am wondering why it is not OK?  Is there any case that the renumbering 
> will cause issue?

If we renumber kernel interface enum, OVS does not work if there is mismatch 
userspace and ovs kernel module versions. So to maintain compatibility we add 
new enum at the end.

Wenyu: Thanks for your answering. It sounds reasonable. 
But  when userpace and kernel module versions mismatches, OVS may not work well 
even though the old kernel interface enum not changed due to the userland or 
kernel module data path may be changed.  
Keeping the existent kernel interface enum unchanged may reduce the possibility 
that OVS doesn't work when the userpace and kernel module versions mismatches, 
but can not avoid the possibility totally. Is that true?

> Thanks a lot.
>
>> @@ -338,10 +345,12 @@ enum ovs_tunnel_key_attr {
>>         OVS_TUNNEL_KEY_ATTR_IPV4_DST,           /* be32 dst IP address. */
>>         OVS_TUNNEL_KEY_ATTR_TOS,                /* u8 Tunnel IP ToS. */
>>         OVS_TUNNEL_KEY_ATTR_TTL,                /* u8 Tunnel IP TTL. */
>>         OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT,      /* No argument, set DF. */
>>         OVS_TUNNEL_KEY_ATTR_CSUM,               /* No argument. CSUM packet. 
>> */
>> +       OVS_TUNNEL_KEY_ATTR_TP_SRC,             /* be16 src Transport Port. 
>> */
>> +       OVS_TUNNEL_KEY_ATTR_TP_DST,             /* be16 dst Transport Port. 
>> */
>>         OVS_TUNNEL_KEY_ATTR_OAM,                /* No argument, OAM frame. */
>>         OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS,        /* Array of Geneve options */
>>         __OVS_TUNNEL_KEY_ATTR_MAX
>>  };
>>  #define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1)
>>
>> I'm continuing to look at it.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to