diff mbox series

[10/20] net/cnxk: added Rx metadata negotiate operation

Message ID 20220207072932.22409-10-ndabilpuram@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series [01/20] common/cnxk: increase resource count for bitmap alloc |

Commit Message

Nithin Dabilpuram Feb. 7, 2022, 7:29 a.m. UTC 
  From: Satha Rao <skoteshwar@marvell.com>

Added rx_metadata_negotiate api to enable mark update RX offload.
Removed software logic to enable/disable mark update inside flow
create/destroy apis.

Signed-off-by: Satha Rao <skoteshwar@marvell.com>
---
 drivers/net/cnxk/cn10k_ethdev.c   | 26 ++++++++++++++++++++++++++
 drivers/net/cnxk/cn10k_rte_flow.c | 20 ++------------------
 drivers/net/cnxk/cn9k_ethdev.c    | 25 +++++++++++++++++++++++++
 drivers/net/cnxk/cn9k_rte_flow.c  | 20 ++------------------
 drivers/net/cnxk/cnxk_ethdev.h    |  1 +
 5 files changed, 56 insertions(+), 36 deletions(-)

Comments

Jerin Jacob Feb. 17, 2022, 1:33 p.m. UTC | #1 
On Mon, Feb 7, 2022 at 1:01 PM Nithin Dabilpuram
<ndabilpuram@marvell.com> wrote:
>
> From: Satha Rao <skoteshwar@marvell.com>
>
> Added rx_metadata_negotiate api to enable mark update RX offload.
> Removed software logic to enable/disable mark update inside flow
> create/destroy apis.

APIs

> Signed-off-by: Satha Rao <skoteshwar@marvell.com>
> ---

> +static int
> +cn10k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
> +{
> +       struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
> +
> +       *features &=
> +               (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
> +
> +       if (*features) {
> +               dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
> +               dev->rx_mark_update = true;
> +       } else {
> +               dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
> +               dev->rx_mark_update = false;
> +       }
> +
> +       cn10k_eth_set_rx_function(eth_dev);
> +
> +       return 0;
> +}

See below.

>
> +static int
> +cn9k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
> +{
> +       struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
> +
> +       *features &=
> +               (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
> +
> +       if (*features) {
> +               dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
> +               dev->rx_mark_update = true;
> +       } else {
> +               dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
> +               dev->rx_mark_update = false;
> +       }
> +
> +       cn9k_eth_set_rx_function(eth_dev);
> +
> +       return 0;
> +}
> +

The above two functions are duplicates, Please pass the function
pointer and make it as one function
for common code.
Nithin Dabilpuram Feb. 22, 2022, 6:31 p.m. UTC | #2 
Please see inline.

Thanks
Nithin

On 2/17/22 7:03 PM, Jerin Jacob wrote:
> On Mon, Feb 7, 2022 at 1:01 PM Nithin Dabilpuram
> <ndabilpuram@marvell.com> wrote:
>>
>> From: Satha Rao <skoteshwar@marvell.com>
>>
>> Added rx_metadata_negotiate api to enable mark update RX offload.
>> Removed software logic to enable/disable mark update inside flow
>> create/destroy apis.
> 
> APIs

Ack.
> 
>> Signed-off-by: Satha Rao <skoteshwar@marvell.com>
>> ---
> 
>> +static int
>> +cn10k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
>> +{
>> +       struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
>> +
>> +       *features &=
>> +               (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
>> +
>> +       if (*features) {
>> +               dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = true;
>> +       } else {
>> +               dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = false;
>> +       }
>> +
>> +       cn10k_eth_set_rx_function(eth_dev);
>> +
>> +       return 0;
>> +}
> 
> See below.
> 
>>
>> +static int
>> +cn9k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
>> +{
>> +       struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
>> +
>> +       *features &=
>> +               (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
>> +
>> +       if (*features) {
>> +               dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = true;
>> +       } else {
>> +               dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = false;
>> +       }
>> +
>> +       cn9k_eth_set_rx_function(eth_dev);
>> +
>> +       return 0;
>> +}
>> +
> 
> The above two functions are duplicates, Please pass the function
> pointer and make it as one function
> for common code.

Though the two functions are same, the fast path offload flags used 
within them are defined per platform so that in future CN9K and CN10K
can have different set of fast path offloads though the two hash defines
are same now. They are also in mutually exclusive files.
diff mbox series

Patch

diff --git a/drivers/net/cnxk/cn10k_ethdev.c b/drivers/net/cnxk/cn10k_ethdev.c
index 8378cbf..169e70e 100644
--- a/drivers/net/cnxk/cn10k_ethdev.c
+++ b/drivers/net/cnxk/cn10k_ethdev.c
@@ -39,6 +39,9 @@  nix_rx_offload_flags(struct rte_eth_dev *eth_dev)
 	if (dev->rx_offloads & RTE_ETH_RX_OFFLOAD_SECURITY)
 		flags |= NIX_RX_OFFLOAD_SECURITY_F;
 
+	if (dev->rx_mark_update)
+		flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
+
 	return flags;
 }
 
@@ -470,6 +473,27 @@  cn10k_nix_dev_start(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
+static int
+cn10k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
+{
+	struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
+
+	*features &=
+		(RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
+
+	if (*features) {
+		dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
+		dev->rx_mark_update = true;
+	} else {
+		dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
+		dev->rx_mark_update = false;
+	}
+
+	cn10k_eth_set_rx_function(eth_dev);
+
+	return 0;
+}
+
 /* Update platform specific eth dev ops */
 static void
 nix_eth_dev_ops_override(void)
@@ -489,6 +513,8 @@  nix_eth_dev_ops_override(void)
 	cnxk_eth_dev_ops.dev_ptypes_set = cn10k_nix_ptypes_set;
 	cnxk_eth_dev_ops.timesync_enable = cn10k_nix_timesync_enable;
 	cnxk_eth_dev_ops.timesync_disable = cn10k_nix_timesync_disable;
+	cnxk_eth_dev_ops.rx_metadata_negotiate =
+		cn10k_nix_rx_metadata_negotiate;
 }
 
 static void
diff --git a/drivers/net/cnxk/cn10k_rte_flow.c b/drivers/net/cnxk/cn10k_rte_flow.c
index 529fb0e..ea71efa 100644
--- a/drivers/net/cnxk/cn10k_rte_flow.c
+++ b/drivers/net/cnxk/cn10k_rte_flow.c
@@ -131,9 +131,9 @@  cn10k_flow_create(struct rte_eth_dev *eth_dev, const struct rte_flow_attr *attr,
 	const struct rte_flow_action *action_rss = NULL;
 	const struct rte_flow_action_meter *mtr = NULL;
 	const struct rte_flow_action *act_q = NULL;
-	int mark_actions = 0, vtag_actions = 0;
 	struct roc_npc *npc = &dev->npc;
 	struct roc_npc_flow *flow;
+	int vtag_actions = 0;
 	uint32_t req_act = 0;
 	int i, rc;
 
@@ -197,13 +197,6 @@  cn10k_flow_create(struct rte_eth_dev *eth_dev, const struct rte_flow_attr *attr,
 			cn10k_mtr_connect(eth_dev, mtr->mtr_id);
 	}
 
-	mark_actions = roc_npc_mark_actions_get(npc);
-
-	if (mark_actions) {
-		dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
-		cn10k_eth_set_rx_function(eth_dev);
-	}
-
 	vtag_actions = roc_npc_vtag_actions_get(npc);
 
 	if (vtag_actions) {
@@ -220,20 +213,11 @@  cn10k_flow_destroy(struct rte_eth_dev *eth_dev, struct rte_flow *rte_flow,
 {
 	struct roc_npc_flow *flow = (struct roc_npc_flow *)rte_flow;
 	struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
-	int mark_actions = 0, vtag_actions = 0;
 	struct roc_npc *npc = &dev->npc;
+	int vtag_actions = 0;
 	uint32_t mtr_id;
 	int rc;
 
-	mark_actions = roc_npc_mark_actions_get(npc);
-	if (mark_actions) {
-		mark_actions = roc_npc_mark_actions_sub_return(npc, 1);
-		if (mark_actions == 0) {
-			dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
-			cn10k_eth_set_rx_function(eth_dev);
-		}
-	}
-
 	vtag_actions = roc_npc_vtag_actions_get(npc);
 	if (vtag_actions) {
 		if (flow->nix_intf == ROC_NPC_INTF_RX) {
diff --git a/drivers/net/cnxk/cn9k_ethdev.c b/drivers/net/cnxk/cn9k_ethdev.c
index d34bc68..c44b5b4 100644
--- a/drivers/net/cnxk/cn9k_ethdev.c
+++ b/drivers/net/cnxk/cn9k_ethdev.c
@@ -39,6 +39,9 @@  nix_rx_offload_flags(struct rte_eth_dev *eth_dev)
 	if (dev->rx_offloads & RTE_ETH_RX_OFFLOAD_SECURITY)
 		flags |= NIX_RX_OFFLOAD_SECURITY_F;
 
+	if (dev->rx_mark_update)
+		flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
+
 	return flags;
 }
 
@@ -467,6 +470,27 @@  cn9k_nix_dev_start(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
+static int
+cn9k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
+{
+	struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
+
+	*features &=
+		(RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
+
+	if (*features) {
+		dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
+		dev->rx_mark_update = true;
+	} else {
+		dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
+		dev->rx_mark_update = false;
+	}
+
+	cn9k_eth_set_rx_function(eth_dev);
+
+	return 0;
+}
+
 /* Update platform specific eth dev ops */
 static void
 nix_eth_dev_ops_override(void)
@@ -487,6 +511,7 @@  nix_eth_dev_ops_override(void)
 	cnxk_eth_dev_ops.timesync_enable = cn9k_nix_timesync_enable;
 	cnxk_eth_dev_ops.timesync_disable = cn9k_nix_timesync_disable;
 	cnxk_eth_dev_ops.mtr_ops_get = NULL;
+	cnxk_eth_dev_ops.rx_metadata_negotiate = cn9k_nix_rx_metadata_negotiate;
 }
 
 static void
diff --git a/drivers/net/cnxk/cn9k_rte_flow.c b/drivers/net/cnxk/cn9k_rte_flow.c
index b94d29e..7b98eb0 100644
--- a/drivers/net/cnxk/cn9k_rte_flow.c
+++ b/drivers/net/cnxk/cn9k_rte_flow.c
@@ -13,21 +13,14 @@  cn9k_flow_create(struct rte_eth_dev *eth_dev, const struct rte_flow_attr *attr,
 		 struct rte_flow_error *error)
 {
 	struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
-	int mark_actions = 0, vtag_actions = 0;
 	struct roc_npc *npc = &dev->npc;
 	struct roc_npc_flow *flow;
+	int vtag_actions = 0;
 
 	flow = cnxk_flow_create(eth_dev, attr, pattern, actions, error);
 	if (!flow)
 		return NULL;
 
-	mark_actions = roc_npc_mark_actions_get(npc);
-
-	if (mark_actions) {
-		dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
-		cn9k_eth_set_rx_function(eth_dev);
-	}
-
 	vtag_actions = roc_npc_vtag_actions_get(npc);
 
 	if (vtag_actions) {
@@ -44,17 +37,8 @@  cn9k_flow_destroy(struct rte_eth_dev *eth_dev, struct rte_flow *rte_flow,
 {
 	struct roc_npc_flow *flow = (struct roc_npc_flow *)rte_flow;
 	struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
-	int mark_actions = 0, vtag_actions = 0;
 	struct roc_npc *npc = &dev->npc;
-
-	mark_actions = roc_npc_mark_actions_get(npc);
-	if (mark_actions) {
-		mark_actions = roc_npc_mark_actions_sub_return(npc, 1);
-		if (mark_actions == 0) {
-			dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
-			cn9k_eth_set_rx_function(eth_dev);
-		}
-	}
+	int vtag_actions = 0;
 
 	vtag_actions = roc_npc_vtag_actions_get(npc);
 	if (vtag_actions) {
diff --git a/drivers/net/cnxk/cnxk_ethdev.h b/drivers/net/cnxk/cnxk_ethdev.h
index fadc8aa..f1363af 100644
--- a/drivers/net/cnxk/cnxk_ethdev.h
+++ b/drivers/net/cnxk/cnxk_ethdev.h
@@ -339,6 +339,7 @@  struct cnxk_eth_dev {
 	uint8_t ptype_disable;
 	bool scalar_ena;
 	bool ptp_en;
+	bool rx_mark_update; /* Enable/Disable mark update to mbuf */
 
 	/* Pointer back to rte */
 	struct rte_eth_dev *eth_dev;