>-----Original Message-----
>From: Ilya Maximets <[email protected]>
>Sent: Thursday, August 13, 2020 9:26 PM
>To: Emma Finn <[email protected]>; [email protected]
>Cc: [email protected]; [email protected]; Eli Britstein <[email protected]>;
>[email protected]; [email protected]
>Subject: Re: [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching
>HWOL for XL710 NIC
>
>
>On 8/13/20 6:13 PM, Emma Finn wrote:
>> This patch introduces a temporary work around to fix partial hardware
>> offload for XL710 devices. Currently the incorrect ethernet pattern is
>> being set. This patch will be removed once this issue is fixed within
>> the i40e PMD.
>>
>> Signed-off-by: Emma Finn <[email protected]>
>> Signed-off-by: Eli Britstein <[email protected]>
>> Co-authored-by: Eli Britstein <[email protected]>
>> ---
>> lib/netdev-offload-dpdk.c | 35 +++++++++++++++++++++++++----------
>> 1 file changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index de6101e..ede2077 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -672,10 +672,24 @@ free_flow_actions(struct flow_actions *actions)
>> actions->cnt = 0;
>> }
>>
>> +/*
>> +* This is a temporary work around to fix ethernet pattern for partial
>> +hardware
>> +* offload for X710 devices. This fix will be reverted once the issue
>> +is fixed
>> +* within the i40e PMD driver.
>> +*/
>> +#define NULL_ETH_MASK_IF_ZEROS \
>> + if (eth_mask && is_all_zeros(ð_mask->src, sizeof eth_mask->src) && \
>> + is_all_zeros(ð_mask->dst, sizeof eth_mask->dst)) { \
>> + patterns->items[0].mask = NULL; \
>> + free(eth_mask); \
>> + }
>
>Could you please address my comments from this e-mail:
>
>https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.op
>envswitch.org%2Fpipermail%2Fovs-dev%2F2020-
>August%2F373873.html&data=02%7C01%7Celibr%40nvidia.com%7C08c8f
>a0338e14858343d08d83fb664bf%7C43083d15727340c1b7db39efd9ccc17a%
>7C0%7C1%7C637329399898256323&sdata=bM8dq1Da0%2F%2FL7m7y
>m470T1aClwH9ZE%2BhlNcED1m7sdQ%3D&reserved=0
>?
>
>i.e. convert this macro definition into function.
My previous patch was a POC. Here is a more "nicer" one. No macro/function, and
no redundant allocation/free:
Need to test and obviously finalize.
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index de6101e4d..c6d293af3 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -696,16 +696,26 @@ parse_flow_match(struct flow_patterns *patterns,
!eth_addr_is_zero(match->wc.masks.dl_dst)) {
struct rte_flow_item_eth *spec, *mask;
- spec = xzalloc(sizeof *spec);
- mask = xzalloc(sizeof *mask);
+ /* WRITE A COMMENT ABOUT THIS WORKAROUND. */
+ if (is_all_zeros(&match->wc.masks.dl_dst, sizeof mask->dst) &&
+ is_all_zeros(&match->wc.masks.dl_src, sizeof mask->src) &&
+ match->wc.masks.dl_type == 0xFFFF &&
+ (match->flow.dl_type == htons(ETH_TYPE_IP) ||
+ match->flow.dl_type == htons(ETH_TYPE_IPV6))) {
+ spec = NULL;
+ mask = NULL;
+ } else {
+ spec = xzalloc(sizeof *spec);
+ mask = xzalloc(sizeof *mask);
- memcpy(&spec->dst, &match->flow.dl_dst, sizeof spec->dst);
- memcpy(&spec->src, &match->flow.dl_src, sizeof spec->src);
- spec->type = match->flow.dl_type;
+ memcpy(&spec->dst, &match->flow.dl_dst, sizeof spec->dst);
+ memcpy(&spec->src, &match->flow.dl_src, sizeof spec->src);
+ spec->type = match->flow.dl_type;
- memcpy(&mask->dst, &match->wc.masks.dl_dst, sizeof mask->dst);
- memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src);
- mask->type = match->wc.masks.dl_type;
+ memcpy(&mask->dst, &match->wc.masks.dl_dst, sizeof mask->dst);
+ memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src);
+ mask->type = match->wc.masks.dl_type;
+ }
memset(&consumed_masks->dl_dst, 0, sizeof consumed_masks->dl_dst);
memset(&consumed_masks->dl_src, 0, sizeof consumed_masks->dl_src);
>
>> +
>> static int
>> parse_flow_match(struct flow_patterns *patterns,
>> struct match *match) {
>> + struct rte_flow_item_eth *eth_mask = NULL;
>> + struct rte_flow_item_eth *eth_spec = NULL;
>> uint8_t *next_proto_mask = NULL;
>> struct flow *consumed_masks;
>> uint8_t proto = 0;
>> @@ -694,24 +708,23 @@ parse_flow_match(struct flow_patterns *patterns,
>> if (match->wc.masks.dl_type ||
>> !eth_addr_is_zero(match->wc.masks.dl_src) ||
>> !eth_addr_is_zero(match->wc.masks.dl_dst)) {
>> - struct rte_flow_item_eth *spec, *mask;
>>
>> - spec = xzalloc(sizeof *spec);
>> - mask = xzalloc(sizeof *mask);
>> + eth_spec = xzalloc(sizeof *eth_spec);
>> + eth_mask = xzalloc(sizeof *eth_mask);
>>
>> - memcpy(&spec->dst, &match->flow.dl_dst, sizeof spec->dst);
>> - memcpy(&spec->src, &match->flow.dl_src, sizeof spec->src);
>> - spec->type = match->flow.dl_type;
>> + memcpy(ð_spec->dst, &match->flow.dl_dst, sizeof eth_spec->dst);
>> + memcpy(ð_spec->src, &match->flow.dl_src, sizeof eth_spec->src);
>> + eth_spec->type = match->flow.dl_type;
>>
>> - memcpy(&mask->dst, &match->wc.masks.dl_dst, sizeof mask->dst);
>> - memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src);
>> - mask->type = match->wc.masks.dl_type;
>> + memcpy(ð_mask->dst, &match->wc.masks.dl_dst, sizeof eth_mask-
>>dst);
>> + memcpy(ð_mask->src, &match->wc.masks.dl_src, sizeof eth_mask-
>>src);
>> + eth_mask->type = match->wc.masks.dl_type;
>>
>> memset(&consumed_masks->dl_dst, 0, sizeof consumed_masks->dl_dst);
>> memset(&consumed_masks->dl_src, 0, sizeof consumed_masks->dl_src);
>> consumed_masks->dl_type = 0;
>>
>> - add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
>> + add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, eth_spec,
>> + eth_mask);
>> }
>>
>> /* VLAN */
>> @@ -738,6 +751,7 @@ parse_flow_match(struct flow_patterns *patterns,
>> /* IP v4 */
>> if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
>> struct rte_flow_item_ipv4 *spec, *mask;
>
>Empty line needed.
>
>> + NULL_ETH_MASK_IF_ZEROS;
>>
>> spec = xzalloc(sizeof *spec);
>> mask = xzalloc(sizeof *mask); @@ -776,6 +790,7 @@
>> parse_flow_match(struct flow_patterns *patterns,
>> /* IP v6 */
>> if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) {
>> struct rte_flow_item_ipv6 *spec, *mask;
>
>ditto.
>
>> + NULL_ETH_MASK_IF_ZEROS;
>>
>> spec = xzalloc(sizeof *spec);
>> mask = xzalloc(sizeof *mask);
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
