From: Alexander Kozyrev Date: Wed, 16 Jun 2021 14:42:36 +0000 (+0300) Subject: net/mlx5: fix modify field action order for MAC X-Git-Url: http://git.droids-corp.org/?a=commitdiff_plain;h=fdd0c046f4;p=dpdk.git net/mlx5: fix modify field action order for MAC MAC addresses are split into 2 parts inside Mellanox NIC: bits 0-15 are separate from bits 16-47. That makes a copy from another packet field tricky because any other field is aligned to 32 bits, not 16. This causes unexpected results when using the MODIFY_FIELD action with MAC addresses. Track crossing MAC addresses boundary and arrange a proper order for the MODIFY_FIELD action involving MAC addresses. Fixes: 641dbe4fb053 ("net/mlx5: support modify field flow action") Cc: stable@dpdk.org Signed-off-by: Alexander Kozyrev Acked-by: Viacheslav Ovsiienko --- diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 7988c773d7..a4a42f455d 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -426,6 +426,8 @@ flow_dv_convert_modify_action(struct rte_flow_item *item, unsigned int off_b; uint32_t mask; uint32_t data; + bool next_field = true; + bool next_dcopy = true; if (i >= MLX5_MAX_MODIFY_NUM) return rte_flow_error_set(error, EINVAL, @@ -443,15 +445,13 @@ flow_dv_convert_modify_action(struct rte_flow_item *item, size_b = sizeof(uint32_t) * CHAR_BIT - off_b - __builtin_clz(mask); MLX5_ASSERT(size_b); - size_b = size_b == sizeof(uint32_t) * CHAR_BIT ? 0 : size_b; actions[i] = (struct mlx5_modification_cmd) { .action_type = type, .field = field->id, .offset = off_b, - .length = size_b, + .length = (size_b == sizeof(uint32_t) * CHAR_BIT) ? + 0 : size_b, }; - /* Convert entire record to expected big-endian format. */ - actions[i].data0 = rte_cpu_to_be_32(actions[i].data0); if (type == MLX5_MODIFICATION_TYPE_COPY) { MLX5_ASSERT(dcopy); actions[i].dst_field = dcopy->id; @@ -459,7 +459,27 @@ flow_dv_convert_modify_action(struct rte_flow_item *item, (int)dcopy->offset < 0 ? off_b : dcopy->offset; /* Convert entire record to big-endian format. */ actions[i].data1 = rte_cpu_to_be_32(actions[i].data1); - ++dcopy; + /* + * Destination field overflow. Copy leftovers of + * a source field to the next destination field. + */ + if ((size_b > dcopy->size * CHAR_BIT) && dcopy->size) { + actions[i].length = dcopy->size * CHAR_BIT; + field->offset += dcopy->size; + next_field = false; + } + /* + * Not enough bits in a source filed to fill a + * destination field. Switch to the next source. + */ + if (dcopy->size > field->size && + (size_b == field->size * CHAR_BIT)) { + actions[i].length = field->size * CHAR_BIT; + dcopy->offset += field->size * CHAR_BIT; + next_dcopy = false; + } + if (next_dcopy) + ++dcopy; } else { MLX5_ASSERT(item->spec); data = flow_dv_fetch_field((const uint8_t *)item->spec + @@ -468,8 +488,11 @@ flow_dv_convert_modify_action(struct rte_flow_item *item, data = (data & mask) >> off_b; actions[i].data1 = rte_cpu_to_be_32(data); } + /* Convert entire record to expected big-endian format. */ + actions[i].data0 = rte_cpu_to_be_32(actions[i].data0); + if (next_field) + ++field; ++i; - ++field; } while (field->size); if (resource->actions_num == i) return rte_flow_error_set(error, EINVAL, @@ -1433,6 +1456,7 @@ mlx5_flow_field_id_to_modify_info struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_dev_config *config = &priv->config; uint32_t idx = 0; + uint32_t off = 0; uint64_t val = 0; switch (data->field) { case RTE_FLOW_FIELD_START: @@ -1440,61 +1464,63 @@ mlx5_flow_field_id_to_modify_info MLX5_ASSERT(false); break; case RTE_FLOW_FIELD_MAC_DST: + off = data->offset > 16 ? data->offset - 16 : 0; if (mask) { - if (data->offset < 32) { - info[idx] = (struct field_modify_info){4, 0, - MLX5_MODI_OUT_DMAC_47_16}; - if (width < 32) { - mask[idx] = - rte_cpu_to_be_32(0xffffffff >> - (32 - width)); + if (data->offset < 16) { + info[idx] = (struct field_modify_info){2, 0, + MLX5_MODI_OUT_DMAC_15_0}; + if (width < 16) { + mask[idx] = rte_cpu_to_be_16(0xffff >> + (16 - width)); width = 0; } else { - mask[idx] = RTE_BE32(0xffffffff); - width -= 32; + mask[idx] = RTE_BE16(0xffff); + width -= 16; } if (!width) break; ++idx; } - info[idx] = (struct field_modify_info){2, 4 * idx, - MLX5_MODI_OUT_DMAC_15_0}; - mask[idx] = rte_cpu_to_be_16(0xffff >> (16 - width)); - } else { - if (data->offset < 32) - info[idx++] = (struct field_modify_info){4, 0, + info[idx] = (struct field_modify_info){4, 4 * idx, MLX5_MODI_OUT_DMAC_47_16}; - info[idx] = (struct field_modify_info){2, 0, + mask[idx] = rte_cpu_to_be_32((0xffffffff >> + (32 - width)) << off); + } else { + if (data->offset < 16) + info[idx++] = (struct field_modify_info){2, 0, MLX5_MODI_OUT_DMAC_15_0}; + info[idx] = (struct field_modify_info){4, off, + MLX5_MODI_OUT_DMAC_47_16}; } break; case RTE_FLOW_FIELD_MAC_SRC: + off = data->offset > 16 ? data->offset - 16 : 0; if (mask) { - if (data->offset < 32) { - info[idx] = (struct field_modify_info){4, 0, - MLX5_MODI_OUT_SMAC_47_16}; - if (width < 32) { - mask[idx] = - rte_cpu_to_be_32(0xffffffff >> - (32 - width)); + if (data->offset < 16) { + info[idx] = (struct field_modify_info){2, 0, + MLX5_MODI_OUT_SMAC_15_0}; + if (width < 16) { + mask[idx] = rte_cpu_to_be_16(0xffff >> + (16 - width)); width = 0; } else { - mask[idx] = RTE_BE32(0xffffffff); - width -= 32; + mask[idx] = RTE_BE16(0xffff); + width -= 16; } if (!width) break; ++idx; } - info[idx] = (struct field_modify_info){2, 4 * idx, - MLX5_MODI_OUT_SMAC_15_0}; - mask[idx] = rte_cpu_to_be_16(0xffff >> (16 - width)); - } else { - if (data->offset < 32) - info[idx++] = (struct field_modify_info){4, 0, + info[idx] = (struct field_modify_info){4, 4 * idx, MLX5_MODI_OUT_SMAC_47_16}; - info[idx] = (struct field_modify_info){2, 0, + mask[idx] = rte_cpu_to_be_32((0xffffffff >> + (32 - width)) << off); + } else { + if (data->offset < 16) + info[idx++] = (struct field_modify_info){2, 0, MLX5_MODI_OUT_SMAC_15_0}; + info[idx] = (struct field_modify_info){4, off, + MLX5_MODI_OUT_SMAC_47_16}; } break; case RTE_FLOW_FIELD_VLAN_TYPE: @@ -1818,7 +1844,12 @@ mlx5_flow_field_id_to_modify_info val = data->value; for (idx = 0; idx < MLX5_ACT_MAX_MOD_FIELDS; idx++) { if (mask[idx]) { - if (dst_width > 16) { + if (dst_width == 48) { + /*special case for MAC addresses */ + value[idx] = rte_cpu_to_be_16(val); + val >>= 16; + dst_width -= 16; + } else if (dst_width > 16) { value[idx] = rte_cpu_to_be_32(val); val >>= 32; } else if (dst_width > 8) {