diff mbox series

[v5,13/25] net/nfp: support SRC MAC flow action

Message ID 1666232391-29152-14-git-send-email-chaoyong.he@corigine.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series add the basic rte_flow offload support of nfp PMD |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chaoyong He Oct. 20, 2022, 2:19 a.m. UTC 
  Add the corresponding data structure and logics, to support
the offload of set source MAC action.

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 doc/guides/nics/features/nfp.ini         |  1 +
 drivers/net/nfp/flower/nfp_flower_cmsg.h | 27 ++++++++++++++++++
 drivers/net/nfp/nfp_flow.c               | 47 ++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+)

Comments

Ferruh Yigit Oct. 20, 2022, 11:12 a.m. UTC | #1 
On 10/20/2022 3:19 AM, Chaoyong He wrote:
> Add the corresponding data structure and logics, to support
> the offload of set source MAC action.
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>

<...>

> +		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
> +			PMD_DRV_LOG(DEBUG, "Process RTE_FLOW_ACTION_TYPE_SET_MAC_SRC");
> +			nfp_flow_action_set_mac(position, action, true, mac_set_flag);
> +			if (!mac_set_flag) {
> +				position += sizeof(struct nfp_fl_act_set_eth);
> +				mac_set_flag = true;
> +			}

Hi Andrew, Ori,

I can see 'RTE_FLOW_ACTION_TYPE_SET_MAC_SRC' and many  other marked as 
legacy and reference to 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'.

What is the expectation from PMD developers for this?

User still can provide these legacy actions, right? So should PMD 
implement both legacy and new ones?
Chaoyong He Oct. 20, 2022, 11:48 a.m. UTC | #2 
> On 10/20/2022 3:19 AM, Chaoyong He wrote:
> > Add the corresponding data structure and logics, to support the
> > offload of set source MAC action.
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 
> <...>
> 
> > +		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
> > +			PMD_DRV_LOG(DEBUG, "Process
> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC");
> > +			nfp_flow_action_set_mac(position, action, true,
> mac_set_flag);
> > +			if (!mac_set_flag) {
> > +				position += sizeof(struct nfp_fl_act_set_eth);
> > +				mac_set_flag = true;
> > +			}
> 
> Hi Andrew, Ori,
> 
> I can see 'RTE_FLOW_ACTION_TYPE_SET_MAC_SRC' and many  other
> marked as legacy and reference to
> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'.
> 
> What is the expectation from PMD developers for this?
> 
> User still can provide these legacy actions, right? So should PMD implement
> both legacy and new ones?

From my side, we trying to implement the basic offload in this DPDK release, and complete
the missing pieces in the next few DPDK releases.

The support of 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' is in the very next project after
this series.
Ferruh Yigit Oct. 20, 2022, 11:55 a.m. UTC | #3 
On 10/20/2022 12:48 PM, Chaoyong He wrote:
>> On 10/20/2022 3:19 AM, Chaoyong He wrote:
>>> Add the corresponding data structure and logics, to support the
>>> offload of set source MAC action.
>>>
>>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>
>> <...>
>>
>>> +		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
>>> +			PMD_DRV_LOG(DEBUG, "Process
>> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC");
>>> +			nfp_flow_action_set_mac(position, action, true,
>> mac_set_flag);
>>> +			if (!mac_set_flag) {
>>> +				position += sizeof(struct nfp_fl_act_set_eth);
>>> +				mac_set_flag = true;
>>> +			}
>>
>> Hi Andrew, Ori,
>>
>> I can see 'RTE_FLOW_ACTION_TYPE_SET_MAC_SRC' and many  other
>> marked as legacy and reference to
>> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'.
>>
>> What is the expectation from PMD developers for this?
>>
>> User still can provide these legacy actions, right? So should PMD implement
>> both legacy and new ones?
> 
>  From my side, we trying to implement the basic offload in this DPDK release, and complete
> the missing pieces in the next few DPDK releases.
> 
> The support of 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' is in the very next project after
> this series.

Sounds reasonable to me, but also I am not aware of the long term plan 
there, so I need input from Andrew and Ori.
diff mbox series

Patch

diff --git a/doc/guides/nics/features/nfp.ini b/doc/guides/nics/features/nfp.ini
index 27575d1..f7cd070 100644
--- a/doc/guides/nics/features/nfp.ini
+++ b/doc/guides/nics/features/nfp.ini
@@ -40,3 +40,4 @@  vlan                 = Y
 count                = Y
 drop                 = Y
 port_id              = Y
+set_mac_src          = Y
diff --git a/drivers/net/nfp/flower/nfp_flower_cmsg.h b/drivers/net/nfp/flower/nfp_flower_cmsg.h
index 36d406f..b61342e 100644
--- a/drivers/net/nfp/flower/nfp_flower_cmsg.h
+++ b/drivers/net/nfp/flower/nfp_flower_cmsg.h
@@ -335,6 +335,33 @@  struct nfp_fl_act_output {
 	rte_be32_t port;
 };
 
+/*
+ * ETH
+ *    3                   2                   1
+ *  1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |   -   |opcode |   -   |jump_id|               -               |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |                     eth_dst_47_16_mask                        |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |      eth_dst_15_0_mask        |      eth_src_47_32_mask       |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |                     eth_src_31_0_mask                         |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |                     eth_dst_47_16                             |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |      eth_dst_15_0             |      eth_src_47_32            |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |                     eth_src_31_0                              |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
+struct nfp_fl_act_set_eth {
+	struct nfp_fl_act_head head;
+	rte_be16_t reserved;
+	uint8_t eth_addr_mask[RTE_ETHER_ADDR_LEN * 2];
+	uint8_t eth_addr[RTE_ETHER_ADDR_LEN * 2];
+};
+
 int nfp_flower_cmsg_mac_repr(struct nfp_app_fw_flower *app_fw_flower);
 int nfp_flower_cmsg_repr_reify(struct nfp_app_fw_flower *app_fw_flower,
 		struct nfp_flower_representor *repr);
diff --git a/drivers/net/nfp/nfp_flow.c b/drivers/net/nfp/nfp_flow.c
index 35456a5..f40d777 100644
--- a/drivers/net/nfp/nfp_flow.c
+++ b/drivers/net/nfp/nfp_flow.c
@@ -561,6 +561,7 @@  struct nfp_mask_id_entry {
 		struct nfp_fl_key_ls *key_ls)
 {
 	int ret = 0;
+	bool mac_set_flag = false;
 	const struct rte_flow_action *action;
 
 	for (action = actions; action->type != RTE_FLOW_ACTION_TYPE_END; ++action) {
@@ -591,6 +592,13 @@  struct nfp_mask_id_entry {
 			PMD_DRV_LOG(DEBUG, "RTE_FLOW_ACTION_TYPE_PORT_ID detected");
 			key_ls->act_size += sizeof(struct nfp_fl_act_output);
 			break;
+		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
+			PMD_DRV_LOG(DEBUG, "RTE_FLOW_ACTION_TYPE_SET_MAC_SRC detected");
+			if (!mac_set_flag) {
+				key_ls->act_size += sizeof(struct nfp_fl_act_set_eth);
+				mac_set_flag = true;
+			}
+			break;
 		default:
 			PMD_DRV_LOG(ERR, "Action type %d not supported.", action->type);
 			return -ENOTSUP;
@@ -1209,6 +1217,36 @@  struct nfp_mask_id_entry {
 	return 0;
 }
 
+static void
+nfp_flow_action_set_mac(char *act_data,
+		const struct rte_flow_action *action,
+		bool mac_src_flag,
+		bool mac_set_flag)
+{
+	size_t act_size;
+	struct nfp_fl_act_set_eth *set_eth;
+	const struct rte_flow_action_set_mac *set_mac;
+
+	if (mac_set_flag)
+		set_eth = (struct nfp_fl_act_set_eth *)act_data - 1;
+	else
+		set_eth = (struct nfp_fl_act_set_eth *)act_data;
+
+	act_size = sizeof(struct nfp_fl_act_set_eth);
+	set_eth->head.jump_id = NFP_FL_ACTION_OPCODE_SET_ETHERNET;
+	set_eth->head.len_lw  = act_size >> NFP_FL_LW_SIZ;
+	set_eth->reserved     = 0;
+
+	set_mac = (const struct rte_flow_action_set_mac *)action->conf;
+	if (mac_src_flag) {
+		rte_memcpy(&set_eth->eth_addr[RTE_ETHER_ADDR_LEN],
+				set_mac->mac_addr, RTE_ETHER_ADDR_LEN);
+	} else {
+		rte_memcpy(&set_eth->eth_addr[0],
+				set_mac->mac_addr, RTE_ETHER_ADDR_LEN);
+	}
+}
+
 static int
 nfp_flow_compile_action(__rte_unused struct nfp_flower_representor *representor,
 		const struct rte_flow_action actions[],
@@ -1218,6 +1256,7 @@  struct nfp_mask_id_entry {
 	char *position;
 	char *action_data;
 	bool drop_flag = false;
+	bool mac_set_flag = false;
 	uint32_t total_actions = 0;
 	const struct rte_flow_action *action;
 	struct nfp_fl_rule_metadata *nfp_flow_meta;
@@ -1254,6 +1293,14 @@  struct nfp_mask_id_entry {
 
 			position += sizeof(struct nfp_fl_act_output);
 			break;
+		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
+			PMD_DRV_LOG(DEBUG, "Process RTE_FLOW_ACTION_TYPE_SET_MAC_SRC");
+			nfp_flow_action_set_mac(position, action, true, mac_set_flag);
+			if (!mac_set_flag) {
+				position += sizeof(struct nfp_fl_act_set_eth);
+				mac_set_flag = true;
+			}
+			break;
 		default:
 			PMD_DRV_LOG(ERR, "Unsupported action type: %d", action->type);
 			return -ENOTSUP;