Re: [PATCH net-next 2/2] net/mlx5e: add ct_metadata.nat support in ct offload

[Oz Shlomo]

Hi Wenxu,

On 5/28/2020 10:15 AM, we...@ucloud.cn wrote:

From: wenxu <we...@ucloud.cn>

In the ct offload all the conntrack entry offload  rules
will be add to both ct ft and ct_nat ft twice.
It is not makesense. The ct_metadat.nat will tell driver

Adding the connection to both tables is required because the user may
perform a CT action without NAT even though a NAT entry was allocated
when the connection was committed.

the rule should add to ct or ct_nat flow table

Signed-off-by: wenxu <we...@ucloud.cn>
---
  drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c | 34 ++++++++--------------
  1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
index 995b2ef..02ecd24 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
@@ -88,7 +88,7 @@ struct mlx5_ct_entry {
        struct mlx5_fc *counter;
        unsigned long cookie;
        unsigned long restore_cookie;
-       struct mlx5_ct_zone_rule zone_rules[2];
+       struct mlx5_ct_zone_rule zone_rule;
  };

static const struct rhashtable_params cts_ht_params = {

@@ -238,10 +238,9 @@ struct mlx5_ct_entry {

static void

  mlx5_tc_ct_entry_del_rule(struct mlx5_tc_ct_priv *ct_priv,
-                         struct mlx5_ct_entry *entry,
-                         bool nat)
+                         struct mlx5_ct_entry *entry)
  {
-       struct mlx5_ct_zone_rule *zone_rule = &entry->zone_rules[nat];
+       struct mlx5_ct_zone_rule *zone_rule = &entry->zone_rule;
        struct mlx5_esw_flow_attr *attr = &zone_rule->attr;
        struct mlx5_eswitch *esw = ct_priv->esw;

@@ -256,8 +255,7 @@ struct mlx5_ct_entry {

  mlx5_tc_ct_entry_del_rules(struct mlx5_tc_ct_priv *ct_priv,
                           struct mlx5_ct_entry *entry)
  {
-       mlx5_tc_ct_entry_del_rule(ct_priv, entry, true);
-       mlx5_tc_ct_entry_del_rule(ct_priv, entry, false);
+       mlx5_tc_ct_entry_del_rule(ct_priv, entry);

mlx5_fc_destroy(ct_priv->esw->dev, entry->counter);

  }
@@ -493,7 +491,7 @@ struct mlx5_ct_entry {
                          struct mlx5_ct_entry *entry,
                          bool nat)
  {
-       struct mlx5_ct_zone_rule *zone_rule = &entry->zone_rules[nat];
+       struct mlx5_ct_zone_rule *zone_rule = &entry->zone_rule;
        struct mlx5_esw_flow_attr *attr = &zone_rule->attr;
        struct mlx5_eswitch *esw = ct_priv->esw;
        struct mlx5_flow_spec *spec = NULL;
@@ -562,7 +560,8 @@ struct mlx5_ct_entry {
  static int
  mlx5_tc_ct_entry_add_rules(struct mlx5_tc_ct_priv *ct_priv,
                           struct flow_rule *flow_rule,
-                          struct mlx5_ct_entry *entry)
+                          struct mlx5_ct_entry *entry,
+                          bool nat)
  {
        struct mlx5_eswitch *esw = ct_priv->esw;
        int err;
@@ -574,20 +573,10 @@ struct mlx5_ct_entry {
                return err;
        }

- err = mlx5_tc_ct_entry_add_rule(ct_priv, flow_rule, entry, false);

+       err = mlx5_tc_ct_entry_add_rule(ct_priv, flow_rule, entry, nat);
        if (err)
-               goto err_orig;
-
-       err = mlx5_tc_ct_entry_add_rule(ct_priv, flow_rule, entry, true);
-       if (err)
-               goto err_nat;
-
-       return 0;
+               mlx5_fc_destroy(esw->dev, entry->counter);

-err_nat:

-       mlx5_tc_ct_entry_del_rule(ct_priv, entry, false);
-err_orig:
-       mlx5_fc_destroy(esw->dev, entry->counter);
        return err;
  }

@@ -619,7 +608,8 @@ struct mlx5_ct_entry {

        entry->cookie = flow->cookie;
        entry->restore_cookie = meta_action->ct_metadata.cookie;

- err = mlx5_tc_ct_entry_add_rules(ct_priv, flow_rule, entry);

+       err = mlx5_tc_ct_entry_add_rules(ct_priv, flow_rule, entry,
+                                        meta_action->ct_metadata.nat);
        if (err)
                goto err_rules;

@@ -1620,7 +1610,7 @@ struct mlx5_flow_handle *

                return false;

entry = container_of(zone_rule, struct mlx5_ct_entry,

-                            zone_rules[zone_rule->nat]);
+                            zone_rule);
        tcf_ct_flow_table_restore_skb(skb, entry->restore_cookie);

return true;