diff mbox series

[ovs-dev,v8,05/15] netdev-offload-tc: Fix tc conntrack force commit support.

Message ID 167456500870.1023551.12196395263788293499.stgit@ebuild.local
State Changes Requested
Headers show
Series tests: Add system-traffic.at tests to check-offloads. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Eelco Chaudron Jan. 24, 2023, 12:56 p.m. UTC
tc was not setting the OVS_CT_ATTR_FORCE_COMMIT flag when a forced
commit was requested. This patch will fix this.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Roi Dayan <roid@nvidia.com>
---
 lib/netdev-offload-tc.c  |   13 +++++++++++--
 tests/system-offloads.at |    1 -
 2 files changed, 11 insertions(+), 3 deletions(-)

Comments

Ilya Maximets Jan. 26, 2023, 8:40 p.m. UTC | #1
On 1/24/23 13:56, Eelco Chaudron wrote:
> tc was not setting the OVS_CT_ATTR_FORCE_COMMIT flag when a forced
> commit was requested. This patch will fix this.
> 
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> Acked-by: Roi Dayan <roid@nvidia.com>
> ---

Should this have:

Fixes: 576126a931cd ("netdev-offload-tc: Add conntrack support")

?

>  lib/netdev-offload-tc.c  |   13 +++++++++++--
>  tests/system-offloads.at |    1 -
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index ce7f8ad97..915c45ed3 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -825,7 +825,11 @@ parse_tc_flower_to_actions__(struct tc_flower *flower, struct ofpbuf *buf,
>              ct_offset = nl_msg_start_nested(buf, OVS_ACTION_ATTR_CT);
>  
>              if (action->ct.commit) {
> -                nl_msg_put_flag(buf, OVS_CT_ATTR_COMMIT);
> +                if (action->ct.force) {
> +                    nl_msg_put_flag(buf, OVS_CT_ATTR_FORCE_COMMIT);
> +                } else {
> +                    nl_msg_put_flag(buf, OVS_CT_ATTR_COMMIT);
> +                }
>              }
>  
>              if (action->ct.zone) {
> @@ -1309,7 +1313,12 @@ parse_put_flow_ct_action(struct tc_flower *flower,
>          NL_ATTR_FOR_EACH_UNSAFE (ct_attr, ct_left, ct, ct_len) {
>              switch (nl_attr_type(ct_attr)) {
>                  case OVS_CT_ATTR_COMMIT: {
> -                        action->ct.commit = true;
> +                    action->ct.commit = true;
> +                }
> +                break;
> +                case OVS_CT_ATTR_FORCE_COMMIT: {
> +                    action->ct.commit = true;
> +                    action->ct.force = true;
>                  }
>                  break;
>                  case OVS_CT_ATTR_ZONE: {
> diff --git a/tests/system-offloads.at b/tests/system-offloads.at
> index 7b6deccf0..d39997708 100644
> --- a/tests/system-offloads.at
> +++ b/tests/system-offloads.at
> @@ -43,7 +43,6 @@ m4_define([OVS_TEST_SKIP_LIST],
>      [ovs_test_skip_list="
>  datapath - truncate and output to gre tunnel by simulated packets
>  datapath - truncate and output to gre tunnel
> -conntrack - force commit
>  conntrack - preserve registers
>  conntrack - zones
>  conntrack - zones from field
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Eelco Chaudron Jan. 30, 2023, 3:54 p.m. UTC | #2
On 26 Jan 2023, at 21:40, Ilya Maximets wrote:

> On 1/24/23 13:56, Eelco Chaudron wrote:
>> tc was not setting the OVS_CT_ATTR_FORCE_COMMIT flag when a forced
>> commit was requested. This patch will fix this.
>>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> Acked-by: Roi Dayan <roid@nvidia.com>
>> ---
>
> Should this have:
>
> Fixes: 576126a931cd ("netdev-offload-tc: Add conntrack support")


Yes, will add.
diff mbox series

Patch

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index ce7f8ad97..915c45ed3 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -825,7 +825,11 @@  parse_tc_flower_to_actions__(struct tc_flower *flower, struct ofpbuf *buf,
             ct_offset = nl_msg_start_nested(buf, OVS_ACTION_ATTR_CT);
 
             if (action->ct.commit) {
-                nl_msg_put_flag(buf, OVS_CT_ATTR_COMMIT);
+                if (action->ct.force) {
+                    nl_msg_put_flag(buf, OVS_CT_ATTR_FORCE_COMMIT);
+                } else {
+                    nl_msg_put_flag(buf, OVS_CT_ATTR_COMMIT);
+                }
             }
 
             if (action->ct.zone) {
@@ -1309,7 +1313,12 @@  parse_put_flow_ct_action(struct tc_flower *flower,
         NL_ATTR_FOR_EACH_UNSAFE (ct_attr, ct_left, ct, ct_len) {
             switch (nl_attr_type(ct_attr)) {
                 case OVS_CT_ATTR_COMMIT: {
-                        action->ct.commit = true;
+                    action->ct.commit = true;
+                }
+                break;
+                case OVS_CT_ATTR_FORCE_COMMIT: {
+                    action->ct.commit = true;
+                    action->ct.force = true;
                 }
                 break;
                 case OVS_CT_ATTR_ZONE: {
diff --git a/tests/system-offloads.at b/tests/system-offloads.at
index 7b6deccf0..d39997708 100644
--- a/tests/system-offloads.at
+++ b/tests/system-offloads.at
@@ -43,7 +43,6 @@  m4_define([OVS_TEST_SKIP_LIST],
     [ovs_test_skip_list="
 datapath - truncate and output to gre tunnel by simulated packets
 datapath - truncate and output to gre tunnel
-conntrack - force commit
 conntrack - preserve registers
 conntrack - zones
 conntrack - zones from field