[ovs-dev] [PATCH] netdev-offload-tc: use nl_msg_put_flag for OVS_TUNNEL_KEY_ATTR_CSUM

[Ilya Maximets]

On 4/7/21 2:32 PM, Eelco Chaudron wrote:
> 
> 
> On 7 Apr 2021, at 13:43, Paolo Valerio wrote:
> 
>> when a tunnel port gets added to the bridge setting the checksum option
>> to true:
>>
>> ovs-vsctl add-port br0 geneve0 -- set interface geneve0 type=geneve options:remote_ip=<remote_ip> options:key=<key> options:csum=true
>>
>> the flow dump for the outgoing traffic will include a
>> "bad key length 1 ..." message:
>>
>> ovs-appctl dpctl/dump-flows --names -m
>> ufid:1bee60c5-53df-4c65-8e18-f042df400423, skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(ovs-l0),packet_type(ns=0/0,id=0/0),eth(src=8e:31:e4:89:03:30,dst=c2:be:66:7e:12:c7),eth_type(0x0800),ipv4(src=0.0.0.0/0.0.0.0,dst=0.0.0.0/0.0.0.0,proto=0/0,tos=0/0x3,ttl=0/0,frag=no), packets:4, bytes:336, used:0.930s, dp:tc, actions:set(tunnel(tun_id=0x7b,dst=192.168.56.11,ttl=64,tp_dst=6081,key6(bad key length 1, expected 0)(01)flags(key))),genev_sys_6081
>>
>> This is due to a mismatch present between the expected length (zero
>> for OVS_TUNNEL_KEY_ATTR_CSUM in ovs_tun_key_attr_lens) and the
>> current one.
>>
>> With this patch the same flow dump becomes:
>>
>> ovs-appctl dpctl/dump-flows --names -m
>> ufid:b5f638c5-6a06-4ff5-8bbd-a16fc1d24844, skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(ovs-l0),packet_type(ns=0/0,id=0/0),eth(src=8e:31:e4:89:03:30,dst=c2:be:66:7e:12:c7),eth_type(0x0800),ipv4(src=0.0.0.0/0.0.0.0,dst=0.0.0.0/0.0.0.0,proto=0/0,tos=0/0x3,ttl=0/0,frag=no), packets:12, bytes:1008, used:0.510s, dp:tc, actions:set(tunnel(tun_id=0x7b,dst=192.168.56.11,ttl=64,tp_dst=6081,flags(csum|key))),genev_sys_6081
>>
>> Fixes: d9677a1f0eaf ("netdev-tc-offloads: TC csum option is not matched with tunnel configuration")
>> Suggested-by: Ilya Maximets <i.maximets at ovn.org>
>> Signed-off-by: Paolo Valerio <pvalerio at redhat.com>
> 
> Looks good to me, as this is in line with how the kernel module handles this.
> 
> Acked-by: Eelco Chaudron <echaudro at redhat.com>

Thanks!  Applied.  Backported down to 2.12.

Best regards, Ilya Maximets.