| Message ID | e7a2eda57c7ab676366e4a57a647c61c9af17457.1728395410.git.echaudro@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add missing TC flower tunnel match and encap flags. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 08/10/2024 16:52, Eelco Chaudron wrote: > This patch adds "Don't Fragment" (DF) flag support to the encap action, > if supported by the kernel. If the kernel does not support this, > it falls back to the previous behavior of ignoring the DF request. > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- > include/linux/tc_act/tc_tunnel_key.h | 1 + > lib/netdev-offload-tc.c | 16 +++++++++++----- > lib/tc.c | 6 ++++++ > lib/tc.h | 1 + > 4 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/include/linux/tc_act/tc_tunnel_key.h b/include/linux/tc_act/tc_tunnel_key.h > index 17291b90b..a6b8bd59d 100644 > --- a/include/linux/tc_act/tc_tunnel_key.h > +++ b/include/linux/tc_act/tc_tunnel_key.h > @@ -42,6 +42,7 @@ enum { > */ > TCA_TUNNEL_KEY_ENC_TOS, /* u8 */ > TCA_TUNNEL_KEY_ENC_TTL, /* u8 */ > + TCA_TUNNEL_KEY_NO_FRAG, /* flag */ > __TCA_TUNNEL_KEY_MAX, > }; > > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index 53c4245b9..8ff662ccd 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -919,6 +919,9 @@ parse_tc_flower_to_actions__(struct tc_flower *flower, struct ofpbuf *buf, > if (!action->encap.no_csum) { > nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_CSUM); > } > + if (action->encap.dont_fragment) { > + nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT); > + } > ret = parse_tc_flower_vxlan_tun_opts(action, buf); > if (ret) { > return ret; > @@ -1641,11 +1644,14 @@ parse_put_flow_set_action(struct tc_flower *flower, struct tc_action *action, > } > 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. */ > + if (enc_flags_support) { > + action->encap.dont_fragment = true; > + } else { > + /* For kernels not supporting the DF flag, we ignoring the > + * configuration requested by the user. This to keep the old, > + * incorrect behaviour, and allow tunnels to be offloaded by > + * TC with these kernels. */ > + } > } > break; > case OVS_TUNNEL_KEY_ATTR_CSUM: { > diff --git a/lib/tc.c b/lib/tc.c > index 6c853b8e6..175e770d4 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -1229,6 +1229,7 @@ static const struct nl_policy tunnel_key_policy[] = { > [TCA_TUNNEL_KEY_ENC_TTL] = { .type = NL_A_U8, .optional = true, }, > [TCA_TUNNEL_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, }, > [TCA_TUNNEL_KEY_NO_CSUM] = { .type = NL_A_U8, .optional = true, }, > + [TCA_TUNNEL_KEY_NO_FRAG] = { .type = NL_A_FLAG, .optional = true, }, > }; > > static int > @@ -1394,6 +1395,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct tc_flower *flower) > struct nlattr *ttl = tun_attrs[TCA_TUNNEL_KEY_ENC_TTL]; > struct nlattr *tun_opt = tun_attrs[TCA_TUNNEL_KEY_ENC_OPTS]; > struct nlattr *no_csum = tun_attrs[TCA_TUNNEL_KEY_NO_CSUM]; > + struct nlattr *no_frag = tun_attrs[TCA_TUNNEL_KEY_NO_FRAG]; > > action = &flower->actions[flower->action_count++]; > action->type = TC_ACT_ENCAP; > @@ -1411,6 +1413,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct tc_flower *flower) > action->encap.tos = tos ? nl_attr_get_u8(tos) : 0; > action->encap.ttl = ttl ? nl_attr_get_u8(ttl) : 0; > action->encap.no_csum = no_csum ? nl_attr_get_u8(no_csum) : 0; > + action->encap.dont_fragment = no_frag ? true : false; > > err = nl_parse_act_tunnel_opts(tun_opt, action); > if (err) { > @@ -2747,6 +2750,9 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, > nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT, > encap->tp_dst); > } > + if (encap->dont_fragment) { > + nl_msg_put_flag(request, TCA_TUNNEL_KEY_NO_FRAG); > + } > nl_msg_put_act_tunnel_vxlan_opts(request, encap); > nl_msg_put_act_tunnel_geneve_option(request, &encap->data); > nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, encap->no_csum); > diff --git a/lib/tc.h b/lib/tc.h > index 8ec4857b7..32a5cfaf9 100644 > --- a/lib/tc.h > +++ b/lib/tc.h > @@ -222,6 +222,7 @@ struct tc_action_encap { > uint8_t tos; > uint8_t ttl; > uint8_t no_csum; > + bool dont_fragment; > struct { > ovs_be32 ipv4_src; > ovs_be32 ipv4_dst; Acked-by: Roi Dayan <roid@nvidia.com>
diff --git a/include/linux/tc_act/tc_tunnel_key.h b/include/linux/tc_act/tc_tunnel_key.h index 17291b90b..a6b8bd59d 100644 --- a/include/linux/tc_act/tc_tunnel_key.h +++ b/include/linux/tc_act/tc_tunnel_key.h @@ -42,6 +42,7 @@ enum { */ TCA_TUNNEL_KEY_ENC_TOS, /* u8 */ TCA_TUNNEL_KEY_ENC_TTL, /* u8 */ + TCA_TUNNEL_KEY_NO_FRAG, /* flag */ __TCA_TUNNEL_KEY_MAX, }; diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 53c4245b9..8ff662ccd 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -919,6 +919,9 @@ parse_tc_flower_to_actions__(struct tc_flower *flower, struct ofpbuf *buf, if (!action->encap.no_csum) { nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_CSUM); } + if (action->encap.dont_fragment) { + nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT); + } ret = parse_tc_flower_vxlan_tun_opts(action, buf); if (ret) { return ret; @@ -1641,11 +1644,14 @@ parse_put_flow_set_action(struct tc_flower *flower, struct tc_action *action, } 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. */ + if (enc_flags_support) { + action->encap.dont_fragment = true; + } else { + /* For kernels not supporting the DF flag, we ignoring the + * configuration requested by the user. This to keep the old, + * incorrect behaviour, and allow tunnels to be offloaded by + * TC with these kernels. */ + } } break; case OVS_TUNNEL_KEY_ATTR_CSUM: { diff --git a/lib/tc.c b/lib/tc.c index 6c853b8e6..175e770d4 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -1229,6 +1229,7 @@ static const struct nl_policy tunnel_key_policy[] = { [TCA_TUNNEL_KEY_ENC_TTL] = { .type = NL_A_U8, .optional = true, }, [TCA_TUNNEL_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, }, [TCA_TUNNEL_KEY_NO_CSUM] = { .type = NL_A_U8, .optional = true, }, + [TCA_TUNNEL_KEY_NO_FRAG] = { .type = NL_A_FLAG, .optional = true, }, }; static int @@ -1394,6 +1395,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct tc_flower *flower) struct nlattr *ttl = tun_attrs[TCA_TUNNEL_KEY_ENC_TTL]; struct nlattr *tun_opt = tun_attrs[TCA_TUNNEL_KEY_ENC_OPTS]; struct nlattr *no_csum = tun_attrs[TCA_TUNNEL_KEY_NO_CSUM]; + struct nlattr *no_frag = tun_attrs[TCA_TUNNEL_KEY_NO_FRAG]; action = &flower->actions[flower->action_count++]; action->type = TC_ACT_ENCAP; @@ -1411,6 +1413,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct tc_flower *flower) action->encap.tos = tos ? nl_attr_get_u8(tos) : 0; action->encap.ttl = ttl ? nl_attr_get_u8(ttl) : 0; action->encap.no_csum = no_csum ? nl_attr_get_u8(no_csum) : 0; + action->encap.dont_fragment = no_frag ? true : false; err = nl_parse_act_tunnel_opts(tun_opt, action); if (err) { @@ -2747,6 +2750,9 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT, encap->tp_dst); } + if (encap->dont_fragment) { + nl_msg_put_flag(request, TCA_TUNNEL_KEY_NO_FRAG); + } nl_msg_put_act_tunnel_vxlan_opts(request, encap); nl_msg_put_act_tunnel_geneve_option(request, &encap->data); nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, encap->no_csum); diff --git a/lib/tc.h b/lib/tc.h index 8ec4857b7..32a5cfaf9 100644 --- a/lib/tc.h +++ b/lib/tc.h @@ -222,6 +222,7 @@ struct tc_action_encap { uint8_t tos; uint8_t ttl; uint8_t no_csum; + bool dont_fragment; struct { ovs_be32 ipv4_src; ovs_be32 ipv4_dst;
This patch adds "Don't Fragment" (DF) flag support to the encap action, if supported by the kernel. If the kernel does not support this, it falls back to the previous behavior of ignoring the DF request. Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- include/linux/tc_act/tc_tunnel_key.h | 1 + lib/netdev-offload-tc.c | 16 +++++++++++----- lib/tc.c | 6 ++++++ lib/tc.h | 1 + 4 files changed, 19 insertions(+), 5 deletions(-)