Re: [ovs-dev] [PATCH v5 7/9] tc: Add vxlan encap action with gbp option offload
On 23 Jun 2023, at 10:44, Eelco Chaudron wrote:
> On 19 Jun 2023, at 13:56, Roi Dayan wrote:
>
>> From: Gavin Li
>>
>> Add TC offload support for vxlan encap with gbp option
>>
>> Signed-off-by: Gavin Li
>> Reviewed-by: Gavi Teitz
>> Reviewed-by: Roi Dayan
>> Reviewed-by: Simon Horman
>
> Thanks for following up! The changes look good to me, one small nit, but not
> worth sending a new rev (maybe it can be fixed during commit).
Missed one more style issue, see below…
> Acked-by: Eelco Chaudron
>
>> ---
>> acinclude.m4 | 7
>> include/linux/tc_act/tc_tunnel_key.h | 17 +++-
>> lib/netdev-offload-tc.c | 31 ++-
>> lib/odp-util.c | 41 ++--
>> lib/odp-util.h | 3 ++
>> lib/tc.c | 58
>> lib/tc.h | 1 +
>> 7 files changed, 143 insertions(+), 15 deletions(-)
>>
>> diff --git a/acinclude.m4 b/acinclude.m4
>> index ac1eab790041..690a13c25967 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -191,6 +191,13 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
>> [AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_TTL], [1],
>> [Define to 1 if TCA_TUNNEL_KEY_ENC_TTL is available.])])
>>
>> + AC_COMPILE_IFELSE([
>> +AC_LANG_PROGRAM([#include ], [
>> +int x = TCA_TUNNEL_KEY_ENC_OPTS_VXLAN;
>> +])],
>> +[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN], [1],
>> + [Define to 1 if TCA_TUNNEL_KEY_ENC_OPTS_VXLAN is
>> available.])])
>> +
>>AC_COMPILE_IFELSE([
>> AC_LANG_PROGRAM([#include ], [
>> int x = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
>> diff --git a/include/linux/tc_act/tc_tunnel_key.h
>> b/include/linux/tc_act/tc_tunnel_key.h
>> index f13acf17dd75..17291b90bf3c 100644
>> --- a/include/linux/tc_act/tc_tunnel_key.h
>> +++ b/include/linux/tc_act/tc_tunnel_key.h
>> @@ -1,7 +1,7 @@
>> #ifndef __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H
>> #define __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H 1
>>
>> -#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_TTL)
>> +#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN)
>> #include_next
>> #else
>>
>> @@ -53,6 +53,10 @@ enum {
>> * TCA_TUNNEL_KEY_ENC_OPTS_GENEVE
>> * attributes
>> */
>> +TCA_TUNNEL_KEY_ENC_OPTS_VXLAN, /* Nested
>> + * TCA_TUNNEL_KEY_ENC_OPTS_VXLAN
>> + * attributes
>> + */
>> __TCA_TUNNEL_KEY_ENC_OPTS_MAX,
>> };
>>
>> @@ -70,6 +74,15 @@ enum {
>> #define TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX \
>> (__TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX - 1)
>>
>> -#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_TTL */
>> +enum {
>> +TCA_TUNNEL_KEY_ENC_OPT_VXLAN_UNSPEC,
>> +TCA_TUNNEL_KEY_ENC_OPT_VXLAN_GBP, /* u32 */
>> +__TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX,
>> +};
>> +
>> +#define TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX \
>> +(__TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX - 1)
>> +
>> +#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN */
>>
>> #endif /* __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H */
>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>> index 1c97681bc92b..a32ed8021c4b 100644
>> --- a/lib/netdev-offload-tc.c
>> +++ b/lib/netdev-offload-tc.c
>> @@ -668,6 +668,24 @@ static void parse_tc_flower_geneve_opts(struct
>> tc_action *action,
>> nl_msg_end_nested(buf, geneve_off);
>> }
>>
>> +static void
>> +parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
>> + struct ofpbuf *buf)
Indentation is off here.
>> +{
>> +size_t gbp_off;
>> +uint32_t gbp_raw;
>> +
>> +if (!action->encap.gbp.id_present) {
>> +return;
>> +}
>> +
>> +gbp_off = nl_msg_start_nested(buf, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
>> +gbp_raw = odp_encode_gbp_raw(action->encap.gbp.flags,
>> + action->encap.gbp.id);
>> +nl_msg_put_u32(buf, OVS_VXLAN_EXT_GBP, gbp_raw);
>> +nl_msg_end_nested(buf, gbp_off);
>> +}
>> +
>> static void
>> flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
>> {
>> @@ -863,7 +881,7 @@ 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);
>> }
>> -
>> +parse_tc_flower_vxlan_tun_opts(action, buf);
>> parse_tc_flower_geneve_opts(action, buf);
>> nl_msg_end_nested(buf, tunnel_offset);
>> nl_msg_end_nested(buf, set_offset);
>> @@ -1552,6 +1570,7 @@ parse_put_flow_set_action(struct tc_flower *flower,
>> struct tc_action *action,
>>
>> action->type = TC_ACT_ENCAP;
>> action->encap.id_present = false;
>> +action->encap
Re: [ovs-dev] [PATCH v5 7/9] tc: Add vxlan encap action with gbp option offload
On 19 Jun 2023, at 13:56, Roi Dayan wrote:
> From: Gavin Li
>
> Add TC offload support for vxlan encap with gbp option
>
> Signed-off-by: Gavin Li
> Reviewed-by: Gavi Teitz
> Reviewed-by: Roi Dayan
> Reviewed-by: Simon Horman
Thanks for following up! The changes look good to me, one small nit, but not
worth sending a new rev (maybe it can be fixed during commit).
Acked-by: Eelco Chaudron
> ---
> acinclude.m4 | 7
> include/linux/tc_act/tc_tunnel_key.h | 17 +++-
> lib/netdev-offload-tc.c | 31 ++-
> lib/odp-util.c | 41 ++--
> lib/odp-util.h | 3 ++
> lib/tc.c | 58
> lib/tc.h | 1 +
> 7 files changed, 143 insertions(+), 15 deletions(-)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index ac1eab790041..690a13c25967 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -191,6 +191,13 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
> [AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_TTL], [1],
> [Define to 1 if TCA_TUNNEL_KEY_ENC_TTL is available.])])
>
> + AC_COMPILE_IFELSE([
> +AC_LANG_PROGRAM([#include ], [
> +int x = TCA_TUNNEL_KEY_ENC_OPTS_VXLAN;
> +])],
> +[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN], [1],
> + [Define to 1 if TCA_TUNNEL_KEY_ENC_OPTS_VXLAN is
> available.])])
> +
>AC_COMPILE_IFELSE([
> AC_LANG_PROGRAM([#include ], [
> int x = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
> diff --git a/include/linux/tc_act/tc_tunnel_key.h
> b/include/linux/tc_act/tc_tunnel_key.h
> index f13acf17dd75..17291b90bf3c 100644
> --- a/include/linux/tc_act/tc_tunnel_key.h
> +++ b/include/linux/tc_act/tc_tunnel_key.h
> @@ -1,7 +1,7 @@
> #ifndef __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H
> #define __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H 1
>
> -#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_TTL)
> +#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN)
> #include_next
> #else
>
> @@ -53,6 +53,10 @@ enum {
>* TCA_TUNNEL_KEY_ENC_OPTS_GENEVE
>* attributes
>*/
> + TCA_TUNNEL_KEY_ENC_OPTS_VXLAN, /* Nested
> + * TCA_TUNNEL_KEY_ENC_OPTS_VXLAN
> + * attributes
> + */
> __TCA_TUNNEL_KEY_ENC_OPTS_MAX,
> };
>
> @@ -70,6 +74,15 @@ enum {
> #define TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX \
> (__TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX - 1)
>
> -#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_TTL */
> +enum {
> + TCA_TUNNEL_KEY_ENC_OPT_VXLAN_UNSPEC,
> + TCA_TUNNEL_KEY_ENC_OPT_VXLAN_GBP, /* u32 */
> + __TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX,
> +};
> +
> +#define TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX \
> + (__TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX - 1)
> +
> +#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN */
>
> #endif /* __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H */
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 1c97681bc92b..a32ed8021c4b 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -668,6 +668,24 @@ static void parse_tc_flower_geneve_opts(struct tc_action
> *action,
> nl_msg_end_nested(buf, geneve_off);
> }
>
> +static void
> +parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
> + struct ofpbuf *buf)
> +{
> +size_t gbp_off;
> +uint32_t gbp_raw;
> +
> +if (!action->encap.gbp.id_present) {
> +return;
> +}
> +
> +gbp_off = nl_msg_start_nested(buf, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
> +gbp_raw = odp_encode_gbp_raw(action->encap.gbp.flags,
> + action->encap.gbp.id);
> +nl_msg_put_u32(buf, OVS_VXLAN_EXT_GBP, gbp_raw);
> +nl_msg_end_nested(buf, gbp_off);
> +}
> +
> static void
> flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
> {
> @@ -863,7 +881,7 @@ 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);
> }
> -
> +parse_tc_flower_vxlan_tun_opts(action, buf);
> parse_tc_flower_geneve_opts(action, buf);
> nl_msg_end_nested(buf, tunnel_offset);
> nl_msg_end_nested(buf, set_offset);
> @@ -1552,6 +1570,7 @@ parse_put_flow_set_action(struct tc_flower *flower,
> struct tc_action *action,
>
> action->type = TC_ACT_ENCAP;
> action->encap.id_present = false;
> +action->encap.gbp.id_present = false;
> action->encap.no_csum = 1;
> flower->action_count++;
> NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
> @@ -1613,6 +1632,16 @@ parse_put_flow_set_action(struct tc_flower *flower,
>
[ovs-dev] [PATCH v5 7/9] tc: Add vxlan encap action with gbp option offload
From: Gavin Li
Add TC offload support for vxlan encap with gbp option
Signed-off-by: Gavin Li
Reviewed-by: Gavi Teitz
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
acinclude.m4 | 7
include/linux/tc_act/tc_tunnel_key.h | 17 +++-
lib/netdev-offload-tc.c | 31 ++-
lib/odp-util.c | 41 ++--
lib/odp-util.h | 3 ++
lib/tc.c | 58
lib/tc.h | 1 +
7 files changed, 143 insertions(+), 15 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index ac1eab790041..690a13c25967 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -191,6 +191,13 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_TTL], [1],
[Define to 1 if TCA_TUNNEL_KEY_ENC_TTL is available.])])
+ AC_COMPILE_IFELSE([
+AC_LANG_PROGRAM([#include ], [
+int x = TCA_TUNNEL_KEY_ENC_OPTS_VXLAN;
+])],
+[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN], [1],
+ [Define to 1 if TCA_TUNNEL_KEY_ENC_OPTS_VXLAN is available.])])
+
AC_COMPILE_IFELSE([
AC_LANG_PROGRAM([#include ], [
int x = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
diff --git a/include/linux/tc_act/tc_tunnel_key.h
b/include/linux/tc_act/tc_tunnel_key.h
index f13acf17dd75..17291b90bf3c 100644
--- a/include/linux/tc_act/tc_tunnel_key.h
+++ b/include/linux/tc_act/tc_tunnel_key.h
@@ -1,7 +1,7 @@
#ifndef __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H
#define __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H 1
-#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_TTL)
+#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN)
#include_next
#else
@@ -53,6 +53,10 @@ enum {
* TCA_TUNNEL_KEY_ENC_OPTS_GENEVE
* attributes
*/
+ TCA_TUNNEL_KEY_ENC_OPTS_VXLAN, /* Nested
+* TCA_TUNNEL_KEY_ENC_OPTS_VXLAN
+* attributes
+*/
__TCA_TUNNEL_KEY_ENC_OPTS_MAX,
};
@@ -70,6 +74,15 @@ enum {
#define TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX \
(__TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX - 1)
-#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_TTL */
+enum {
+ TCA_TUNNEL_KEY_ENC_OPT_VXLAN_UNSPEC,
+ TCA_TUNNEL_KEY_ENC_OPT_VXLAN_GBP, /* u32 */
+ __TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX,
+};
+
+#define TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX \
+ (__TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX - 1)
+
+#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN */
#endif /* __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H */
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 1c97681bc92b..a32ed8021c4b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -668,6 +668,24 @@ static void parse_tc_flower_geneve_opts(struct tc_action
*action,
nl_msg_end_nested(buf, geneve_off);
}
+static void
+parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
+ struct ofpbuf *buf)
+{
+size_t gbp_off;
+uint32_t gbp_raw;
+
+if (!action->encap.gbp.id_present) {
+return;
+}
+
+gbp_off = nl_msg_start_nested(buf, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
+gbp_raw = odp_encode_gbp_raw(action->encap.gbp.flags,
+ action->encap.gbp.id);
+nl_msg_put_u32(buf, OVS_VXLAN_EXT_GBP, gbp_raw);
+nl_msg_end_nested(buf, gbp_off);
+}
+
static void
flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
{
@@ -863,7 +881,7 @@ 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);
}
-
+parse_tc_flower_vxlan_tun_opts(action, buf);
parse_tc_flower_geneve_opts(action, buf);
nl_msg_end_nested(buf, tunnel_offset);
nl_msg_end_nested(buf, set_offset);
@@ -1552,6 +1570,7 @@ parse_put_flow_set_action(struct tc_flower *flower,
struct tc_action *action,
action->type = TC_ACT_ENCAP;
action->encap.id_present = false;
+action->encap.gbp.id_present = false;
action->encap.no_csum = 1;
flower->action_count++;
NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
@@ -1613,6 +1632,16 @@ 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;
+case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: {
+if (odp_vxlan_tun_opts_from_attr(tun_attr,
+ &action->encap.gbp.id,
+ &action->encap.gbp.flags,
+ &action->en
