On 7/14/22 15:13, Roi Dayan wrote:
>
>
> On 2022-07-14 4:01 PM, Roi Dayan wrote:
>>
>>
>> On 2022-07-14 3:52 PM, Roi Dayan wrote:
>>>
>>>
>>> On 2022-07-14 3:37 PM, Roi Dayan wrote:
>>>>
>>>>
>>>> On 2022-07-13 7:28 PM, Ilya Maximets wrote:
>>>>> On 7/13/22 12:11, Roi Dayan wrote:
>>>>>>
>>>>>>
>>>>>> On 2022-07-13 12:01 PM, Roi Dayan wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2022-07-06 4:58 PM, Roi Dayan wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2022-07-05 1:45 AM, Ilya Maximets wrote:
>>>>>>>>> Current offloading code supports only limited number of tunnel keys
>>>>>>>>> and silently ignores everything it doesn't understand. This is
>>>>>>>>> causing, for example, offloaded ERSPAN tunnels to not work, because
>>>>>>>>> flow is offloaded, but ERSPAN options are not provided to TC.
>>>>>>>>>
>>>>>>>>> There is a number of tunnel keys, which are supported by the
>>>>>>>>> userspace,
>>>>>>>>> but silently ignored during offloading:
>>>>>>>>>
>>>>>>>>> OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT
>>>>>>>>> OVS_TUNNEL_KEY_ATTR_OAM
>>>>>>>>> OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS
>>>>>>>>> OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS
>>>>>>>>>
>>>>>>>>> OVS_TUNNEL_KEY_ATTR_CSUM is kind of supported, but only for actions
>>>>>>>>> and for some reason is set from the tunnel port instead of the
>>>>>>>>> provided action, and not currently supported for the tunnel key in
>>>>>>>>> the match.
>>>>>>>>>
>>>>>>>>> Addig a default case to fail offloading of unknown attributes. For
>>>>>>>>> now explicitly allowing incorrect behavior for the DONT_FRAGMENT flag,
>>>>>>>>> otherwise we'll break all tunnel offloading by default. VXLAN and
>>>>>>>>> ERSPAN options has to fail offloading, because the tunnel will not
>>>>>>>>> work otherwise. OAM is not a default configurations, so failing it
>>>>>>>>> as well. The missing DONT_FRAGMENT flag though should, probably,
>>>>>>>>> cause frequent flow revalidation, but that is not new with this patch.
>>>>>>>>>
>>>>>>>>> Same for the 'match' key, only clearing masks that was actually
>>>>>>>>> consumed, except for the DONT_FRAGMENT and CSUM flags, which are
>>>>>>>>> explicitly allowed and highlighted as broken.
>>>>>>>>>
>>>>>>>>> Also, ttl and tos were incorrectly checked by the value instead of a
>>>>>>>>> mask during the flow key dump. Destination port as well as CSUM
>>>>>>>>> configuration for unknown reason was not taken from the actions list
>>>>>>>>> and were passed via HW offload info.
>>>>>>>>>
>>>>>>>>> Reported-at:
>>>>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fpipermail%2Fovs-dev%2F2022-July%2F395522.html&data=05%7C01%7Croid%40nvidia.com%7Cba6998f8523f4fcf52eb08da64ecba35%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637933265159187181%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=QDm8f%2FFzZE0X7q26i09UI8zaGK0UkD9tcEChI2XYNZ4%3D&reserved=0
>>>>>>>>> Reported-by: Eelco Chaudron <[email protected]>
>>>>>>>>> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put
>>>>>>>>> using tc interface")
>>>>>>>>> Signed-off-by: Ilya Maximets <[email protected]>
>>>>>>>>> ---
>>>>>>>>> lib/dpif-netlink.c | 14 +---------
>>>>>>>>> lib/netdev-offload-tc.c | 57
>>>>>>>>> ++++++++++++++++++++++++++++++++++-------
>>>>>>>>> lib/netdev-offload.h | 3 ---
>>>>>>>>> 3 files changed, 49 insertions(+), 25 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>>>>>>>>> index 06e1e8ca0..1e116b4ad 100644
>>>>>>>>> --- a/lib/dpif-netlink.c
>>>>>>>>> +++ b/lib/dpif-netlink.c
>>>>>>>>> @@ -2237,8 +2237,6 @@ parse_flow_put(struct dpif_netlink *dpif,
>>>>>>>>> struct dpif_flow_put *put)
>>>>>>>>> size_t left;
>>>>>>>>> struct netdev *dev;
>>>>>>>>> struct offload_info info;
>>>>>>>>> - ovs_be16 dst_port = 0;
>>>>>>>>> - uint8_t csum_on = false;
>>>>>>>>> int err;
>>>>>>>>> info.tc_modify_flow_deleted = false;
>>>>>>>>> @@ -2258,10 +2256,9 @@ parse_flow_put(struct dpif_netlink *dpif,
>>>>>>>>> struct dpif_flow_put *put)
>>>>>>>>> return EOPNOTSUPP;
>>>>>>>>> }
>>>>>>>>> - /* Get tunnel dst port */
>>>>>>>>> + /* Check the output port for a tunnel. */
>>>>>>>>> NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) {
>>>>>>>>> if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
>>>>>>>>> - const struct netdev_tunnel_config *tnl_cfg;
>>>>>>>>> struct netdev *outdev;
>>>>>>>>> odp_port_t out_port;
>>>>>>>>> @@ -2271,19 +2268,10 @@ parse_flow_put(struct dpif_netlink *dpif,
>>>>>>>>> struct dpif_flow_put *put)
>>>>>>>>> err = EOPNOTSUPP;
>>>>>>>>> goto out;
>>>>>>>>> }
>>>>>>>>> - tnl_cfg = netdev_get_tunnel_config(outdev);
>>>>>>>>> - if (tnl_cfg && tnl_cfg->dst_port != 0) {
>>>>>>>>> - dst_port = tnl_cfg->dst_port;
>>>>>>>>> - }
>>>>>>>>> - if (tnl_cfg) {
>>>>>>>>> - csum_on = tnl_cfg->csum;
>>>>>>>>> - }
>>>>>>>>> netdev_close(outdev);
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>> - info.tp_dst_port = dst_port;
>>>>>>>>> - info.tunnel_csum_on = csum_on;
>>>>>>>>> info.recirc_id_shared_with_tc = (dpif->user_features
>>>>>>>>> & OVS_DP_F_TC_RECIRC_SHARING);
>>>>>>>>> err = netdev_flow_put(dev, &match,
>>>>>>>>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>>>>>>>>> index 262faf3c6..e62687c82 100644
>>>>>>>>> --- a/lib/netdev-offload-tc.c
>>>>>>>>> +++ b/lib/netdev-offload-tc.c
>>>>>>>>> @@ -805,11 +805,11 @@ parse_tc_flower_to_match(struct tc_flower
>>>>>>>>> *flower,
>>>>>>>>> &flower->key.tunnel.ipv6.ipv6_src,
>>>>>>>>> &flower->mask.tunnel.ipv6.ipv6_src);
>>>>>>>>> }
>>>>>>>>> - if (flower->key.tunnel.tos) {
>>>>>>>>> + if (flower->mask.tunnel.tos) {
>>>>>>>>> match_set_tun_tos_masked(match, flower->key.tunnel.tos,
>>>>>>>>> flower->mask.tunnel.tos);
>>>>>>>>> }
>>>>>>>>> - if (flower->key.tunnel.ttl) {
>>>>>>>>> + if (flower->mask.tunnel.ttl) {
>>>>>>>>> match_set_tun_ttl_masked(match, flower->key.tunnel.ttl,
>>>>>>>>> flower->mask.tunnel.ttl);
>>>>>>>>> }
>>>>>>>>> @@ -1284,9 +1284,11 @@ static int
>>>>>>>>> parse_put_flow_set_action(struct tc_flower *flower, struct
>>>>>>>>> tc_action *action,
>>>>>>>>> const struct nlattr *set, size_t set_len)
>>>>>>>>> {
>>>>>>>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>>>>>>>>> const struct nlattr *tunnel;
>>>>>>>>> const struct nlattr *tun_attr;
>>>>>>>>> size_t tun_left, tunnel_len;
>>>>>>>>> + bool attr_csum_present;
>>>>>>>>> if (nl_attr_type(set) == OVS_KEY_ATTR_MPLS) {
>>>>>>>>> return parse_mpls_set_action(flower, action, set);
>>>>>>>>> @@ -1300,6 +1302,7 @@ parse_put_flow_set_action(struct tc_flower
>>>>>>>>> *flower, struct tc_action *action,
>>>>>>>>> tunnel = nl_attr_get(set);
>>>>>>>>> tunnel_len = nl_attr_get_size(set);
>>>>>>>>> + attr_csum_present = false;
>>>>>>>>> action->type = TC_ACT_ENCAP;
>>>>>>>>> action->encap.id_present = false;
>>>>>>>>> flower->action_count++;
>>>>>>>>> @@ -1326,6 +1329,19 @@ parse_put_flow_set_action(struct tc_flower
>>>>>>>>> *flower, struct tc_action *action,
>>>>>>>>> action->encap.ttl = nl_attr_get_u8(tun_attr);
>>>>>>>>> }
>>>>>>>>> break;
>>>>>>>>> + case OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT: {
>>>>>>>>> + /* XXX: This is wrong! We're ignoring the DF flag
>>>>>>>>> configuration
>>>>>>>>> + * requested by the user. However, TC for now has no
>>>>>>>>> way to pass
>>>>>>>>> + * that flag and it is set by default, meaning tunnel
>>>>>>>>> offloading
>>>>>>>>> + * will not work if 'options:df_default=false' is not
>>>>>>>>> set.
>>>>>>>>> + * Keeping incorrect behavior for now. */
>>>>>>>>> + }
>>>>>>>>> + break;
>>>>>>>>> + case OVS_TUNNEL_KEY_ATTR_CSUM: {
>>>>>>>>> + attr_csum_present = true;
>>>>>>>>> + action->encap.no_csum = !nl_attr_get_u8(tun_attr);
>>>>>>>>> + }
>>>>>>>>> + break;
>>>>>>>>> case OVS_TUNNEL_KEY_ATTR_IPV6_SRC: {
>>>>>>>>> action->encap.ipv6.ipv6_src =
>>>>>>>>> nl_attr_get_in6_addr(tun_attr);
>>>>>>>>> @@ -1350,9 +1366,17 @@ parse_put_flow_set_action(struct tc_flower
>>>>>>>>> *flower, struct tc_action *action,
>>>>>>>>> action->encap.data.present.len =
>>>>>>>>> nl_attr_get_size(tun_attr);
>>>>>>>>> }
>>>>>>>>> break;
>>>>>>>>> + default:
>>>>>>>>> + VLOG_DBG_RL(&rl, "unsupported tunnel key attribute %d",
>>>>>>>>> + nl_attr_type(tun_attr));
>>>>>>>>> + return EOPNOTSUPP;
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>> + if (!attr_csum_present) {
>>>>>>>>> + action->encap.no_csum = 1;
>>>>>>>>> + }
>>>>>>>>> +
>>>>>>>>> return 0;
>>>>>>>>> }
>>>>>>>>> @@ -1448,7 +1472,7 @@ test_key_and_mask(struct match *match)
>>>>>>>>> static void
>>>>>>>>> flower_match_to_tun_opt(struct tc_flower *flower, const struct
>>>>>>>>> flow_tnl *tnl,
>>>>>>>>> - const struct flow_tnl *tnl_mask)
>>>>>>>>> + struct flow_tnl *tnl_mask)
>>>>>>>>> {
>>>>>>>>> struct geneve_opt *opt, *opt_mask;
>>>>>>>>> int len, cnt = 0;
>>>>>>>>> @@ -1472,6 +1496,10 @@ flower_match_to_tun_opt(struct tc_flower
>>>>>>>>> *flower, const struct flow_tnl *tnl,
>>>>>>>>> }
>>>>>>>>> flower->mask.tunnel.metadata.present.len =
>>>>>>>>> tnl->metadata.present.len;
>>>>>>>>> +
>>>>>>>>> + memset(tnl_mask->metadata.opts.gnv, 0,
>>>>>>>>> tnl->metadata.present.len);
>>>>>>>>> + memset(&tnl_mask->metadata.present.len, 0,
>>>>>>>>> + sizeof tnl_mask->metadata.present.len);
>>>>>>>>> }
>>>>>>>>> static void
>>>>>>>>> @@ -1576,7 +1604,7 @@ netdev_tc_flow_put(struct netdev *netdev,
>>>>>>>>> struct match *match,
>>>>>>>>> const struct flow *key = &match->flow;
>>>>>>>>> struct flow *mask = &match->wc.masks;
>>>>>>>>> const struct flow_tnl *tnl = &match->flow.tunnel;
>>>>>>>>> - const struct flow_tnl *tnl_mask = &mask->tunnel;
>>>>>>>>> + struct flow_tnl *tnl_mask = &mask->tunnel;
>>>>>>>>> struct tc_action *action;
>>>>>>>>> bool recirc_act = false;
>>>>>>>>> uint32_t block_id = 0;
>>>>>>>>> @@ -1624,10 +1652,25 @@ netdev_tc_flow_put(struct netdev *netdev,
>>>>>>>>> struct match *match,
>>>>>>>>> flower.mask.tunnel.tos = tnl_mask->ip_tos;
>>>>>>>>> flower.mask.tunnel.ttl = tnl_mask->ip_ttl;
>>>>>>>>> flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ?
>>>>>>>>> tnl_mask->tun_id : 0;
>>>>>>>>> + memset(&tnl_mask->tun_id, 0, sizeof tnl_mask->tun_id);
>>>>>>>>> + memset(&tnl_mask->ip_src, 0, sizeof tnl_mask->ip_src);
>>>>>>>>> + memset(&tnl_mask->ip_dst, 0, sizeof tnl_mask->ip_dst);
>>>>>>>>> + memset(&tnl_mask->ipv6_src, 0, sizeof tnl_mask->ipv6_src);
>>>>>>>>> + memset(&tnl_mask->ipv6_dst, 0, sizeof tnl_mask->ipv6_dst);
>>>>>>>>> + memset(&tnl_mask->ip_tos, 0, sizeof tnl_mask->ip_tos);
>>>>>>>>> + memset(&tnl_mask->ip_ttl, 0, sizeof tnl_mask->ip_ttl);
>>>>>>>>> + tnl_mask->flags &= ~FLOW_TNL_F_KEY;
>>>>>>>>> +
>>>>>>>>> + /* XXX: This is wrong! We're ignoring DF and CSUM flags
>>>>>>>>> configuration
>>>>>>>>> + * requested by the user. However, TC for now has no way to
>>>>>>>>> pass
>>>>>>>>> + * these flags in a flower key and their masks are set by
>>>>>>>>> default,
>>>>>>>>> + * meaning tunnel offloading will not work at all if not
>>>>>>>>> cleared.
>>>>>>>>> + * Keeping incorrect behavior for now. */
>>>>>>>>> + tnl_mask->flags &= ~(FLOW_TNL_F_DONT_FRAGMENT |
>>>>>>>>> FLOW_TNL_F_CSUM);
>>>>>>>>> +
>>>>>>>>> flower_match_to_tun_opt(&flower, tnl, tnl_mask);
>>>>>>>>> flower.tunnel = true;
>>>>>>>>> }
>>>>>>>>> - memset(&mask->tunnel, 0, sizeof mask->tunnel);
>>>>>>>>> flower.key.eth_type = key->dl_type;
>>>>>>>>> flower.mask.eth_type = mask->dl_type;
>>>>>>>>> @@ -1899,10 +1942,6 @@ netdev_tc_flow_put(struct netdev *netdev,
>>>>>>>>> struct match *match,
>>>>>>>>> if (err) {
>>>>>>>>> return err;
>>>>>>>>> }
>>>>>>>>> - if (action->type == TC_ACT_ENCAP) {
>>>>>>>>> - action->encap.tp_dst = info->tp_dst_port;
>>>>>>>>> - action->encap.no_csum = !info->tunnel_csum_on;
>>>>>>>>> - }
>>>>>>>>> } else if (nl_attr_type(nla) ==
>>>>>>>>> OVS_ACTION_ATTR_SET_MASKED) {
>>>>>>>>> const struct nlattr *set = nl_attr_get(nla);
>>>>>>>>> const size_t set_len = nl_attr_get_size(nla);
>>>>>>>>> diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
>>>>>>>>> index 8237a85dd..93eb2df48 100644
>>>>>>>>> --- a/lib/netdev-offload.h
>>>>>>>>> +++ b/lib/netdev-offload.h
>>>>>>>>> @@ -65,9 +65,6 @@ struct netdev_flow_dump {
>>>>>>>>> /* Flow offloading. */
>>>>>>>>> struct offload_info {
>>>>>>>>> - ovs_be16 tp_dst_port; /* Destination port for tunnel in SET
>>>>>>>>> action */
>>>>>>>>> - uint8_t tunnel_csum_on; /* Tunnel header with checksum */
>>>>>>>>> -
>>>>>>>>> bool recirc_id_shared_with_tc; /* Indicates whever tc chains
>>>>>>>>> will be in
>>>>>>>>> * sync with datapath recirc
>>>>>>>>> ids. */
>>>>>>>>
>>>>>>>> Acked-by: Roi Dayan <[email protected]>
>>>>>>>
>>>>>>> hi,
>>>>>>>
>>>>>>> I did ack too early maybe I did some more tests and found dpctl rule I
>>>>>>> could add before this commit now fail with this commit.
>>>>>>>
>>>>>>> I created an ovs bridge with a vxlan port and tried the following:
>>>>>>>
>>>>>>> ovs-appctl dpctl/add-flow
>>>>>>> 'ufid:c5f9a0b1-3399-4436-b742-30825c64a1e5,recirc_id(0),in_port(2),eth_type(0x0800),eth(src=00:00:00:00:00:00/00:00:00:00:00:00,dst=fa:16:3e:2a:4e:23),tunnel(tun_id=0x65,src=10.10.11.3,dst=10.10.11.2,ttl=0/0,tp_dst=4789,flags(+key)),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)'
>>>>>>>
>>>>>>> 'set(tunnel(tun_id=0x66,src=10.10.12.2,dst=10.10.12.3,tp_dst=4789,flags(key))),2'
>>>>>>>
>>>>>>> ovs-vswitchd: updating flow table (Invalid argument)
>>>>>>> ovs-appctl: ovs-vswitchd: server returned an error
>>>>>>>
>>>>>>>
>>>>>>> Other rule like the following that was offloaded to TC before this
>>>>>>> commit now not added to TC. added ovs dbg and got the error in the log.
>>>>>>>
>>>>>>> ovs-appctl dpctl/add-flow
>>>>>>> 'ufid:c5f9a0b1-3399-4436-b742-30825c64a1e5,recirc_id(0),tunnel(tun_id=0x2a,src=2.2.2.3,dst=2.2.2.2,tp_dst=4789,ttl=64/0),in_port(3),eth(src=56:52:2d:21:4d:93,dst=92:c1:04:ce:fd:51),eth_type(0x0800),ipv4(src=1.1.1.1)'
>>>>>>> 2
>>>>>>>
>>>>>>> 2022-07-13T08:59:36.100Z|00053|netdev_offload_tc|DBG|offloading isn't
>>>>>>> supported, unknown attribute
>>>>>>>
>>>>>>> I need to go over the commit again and debug for more detailed info.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Roi
>>>>>>
>>>>>>
>>>>>>
>>>>>> In netdev_tc_flow_put() since you removed the memset on the tunnel
>>>>>> and reset per field you need to reset also tp_src/dst to keep working
>>>>>> and not break at this commit.
>>>>>>
>>>>>> + memset(&tnl_mask->tp_src, 0, sizeof tnl_mask->tp_src);
>>>>>> + memset(&tnl_mask->tp_dst, 0, sizeof tnl_mask->tp_dst);
>>>>>>
>>>>>>
>>>>>> this solves the issues I encountered and my tests pass.
>>>>>>
>>>>>
>>>>> Good catch. Yes, we need to move that part from the patch #2
>>>>> to this one, even though we're not using these masks. We're
>>>>> still using keys themselves, but with exact match instead.
>>>>>
>>>>> Is it the only issue you found with this change?
>>>>
>>>> I encountered another issue.
>>>> I config ovs with vxlan interface and key:flow.
>>>> I set the key using openflow rule and ping between 2 interfaces.
>>>> the encap rule is not being added to tc.
>>>> 2022-07-14T12:34:36.910Z|00001|netdev_offload_tc(handler5)|DBG|offloading
>>>> isn't supported, unknown attribute
>>>>
>>>> I need to some prints and find which attribute like i did for tp_dst/src.
>>>>
>>>> ovs-vsctl add-br brv-1
>>>> ovs-vsctl add-port brv-1 veth1
>>>>
>>>> ovs-vsctl add-port brv-1 vxlan0 -- set interface vxlan0 type=vxlan
>>>> options:local_ip=$local_tun options:remote_ip=$remote_tun options:key=flow
>>>> options:dst_port=4789
>>>>
>>>> ovs-ofctl add-flow brv-1 "table=0,in_port=veth1
>>>> actions=set_field:42->tun_id,vxlan0"
>>>>
>>>> ovs-ofctl add-flow brv-1 "table=0,in_port=vxlan0 actions=veth1"
>>>>
>>>>
>>>
>>> added some prints and its mask->tunnel.tun_id but there is
>>> zeroing of it so not sure why i get to this.
>>>
>>> 2022-07-14T12:43:58.543Z|00001|netdev_offload_tc(handler10)|DBG|offloading
>>> isn't supported, unknown tunnel attribute
>>> 2022-07-14T12:43:58.543Z|00002|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 40 value ff
>>> 2022-07-14T12:43:58.543Z|00003|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 41 value ff
>>> 2022-07-14T12:43:58.543Z|00004|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 42 value ff
>>> 2022-07-14T12:43:58.543Z|00005|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 43 value ff
>>> 2022-07-14T12:43:58.543Z|00006|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 44 value ff
>>> 2022-07-14T12:43:58.543Z|00007|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 45 value ff
>>> 2022-07-14T12:43:58.543Z|00008|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 46 value ff
>>> 2022-07-14T12:43:58.543Z|00009|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 47 value ff
>>> 2022-07-14T12:43:58.543Z|00010|netdev_offload_tc(handler10)|DBG|non zero
>>> offset 48 value 8
>>>
>>
>> its also the tunnel->flags
>> 2022-07-14T13:00:54.863Z|00002|netdev_offload_tc(handler10)|DBG|non zero
>> offset 48 value 8
>>
>>
>> i did set df_default=false like your comment suggested.
>
> ok so found we dont get insite the tunnel if clause.
> but the tunnel mask is filled with dp_dst and flags.
> I have no idea why currently but instead of removing the memset
> of tunnel mask done in this commit I moved it into an else caluse.
> is it ok by you?
>
>
> if (flow_tnl_dst_is_set(&key->tunnel) ||
> flow_tnl_src_is_set(&key->tunnel)) {
> ....
> } else {
> memset(tnl_mask, 0, sizeof *tnl_mask);
> }
>
>
Makes sense. It looks like the issue fixed for rte_flow offload here:
0ef70536eb41 ("netdev-offload-dpdk: Support vxlan encap offload with load
actions.")
So, the soulution could be what you did.
The same can also be fixed by the following patch:
https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
In general, I think, it's an artifact of tunnel metadata being
supplied via action. That doesn't fit nicely into generic flow
translation.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
