On Fri, Jul 06, 2018 at 05:11:31PM -0700, Yongseok Koh wrote:
> On Wed, Jun 27, 2018 at 05:07:51PM +0200, Nelio Laranjeiro wrote:
> > Signed-off-by: Nelio Laranjeiro <nelio.laranje...@6wind.com>
> > ---
>[...]
> > + if (spec) {
> > + memcpy(&mpls.val.label, spec, sizeof(mpls.val.label));
> > + memcpy(&mpls.mask.label, mask, sizeof(mpls.mask.label));
> > + /* Remove unwanted bits from values. */
> > + mpls.val.label &= mpls.mask.label;
> > + }
> > + if (size <= flow_size)
>
> Is it guaranteed flow->cur_verbs isn't null if size fits? Could be obvious but
> just want to make sure.
Yes it is.
> > + mlx5_flow_spec_verbs_add(flow, &mpls, size);
> > + mlx5_flow_layers_update(flow, MLX5_FLOW_LAYER_MPLS);
> > + if (layers & MLX5_FLOW_LAYER_OUTER_L4_UDP)
> > + flow->ptype = RTE_PTYPE_TUNNEL_MPLS_IN_GRE | RTE_PTYPE_L4_UDP;
> > + else
> > + flow->ptype = RTE_PTYPE_TUNNEL_MPLS_IN_GRE;
> > + return size;
> > +#endif /* !HAVE_IBV_DEVICE_MPLS_SUPPORT */
> > + return rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "MPLS is not supported by Verbs, please"
> > + " update.");
> > +}
> > +
> > /**
> > * Validate items provided by the user.
> > *
> > @@ -1650,6 +1722,9 @@ mlx5_flow_items(struct rte_eth_dev *dev,
> > case RTE_FLOW_ITEM_TYPE_GRE:
> > ret = mlx5_flow_item_gre(items, flow, remain, error);
> > break;
>
> #ifdef HAVE_IBV_DEVICE_MPLS_SUPPORT
>
> > + case RTE_FLOW_ITEM_TYPE_MPLS:
> > + ret = mlx5_flow_item_mpls(items, flow, remain, error);
> > + break;
>
> #endif /* !HAVE_IBV_DEVICE_MPLS_SUPPORT */
>
> How about this?
>[...]
It adds another couple of #ifdef #endif and the final output won't help
much the user, having an error "MPLS is not updated by Verbs, please
update" will help more than "item not supported".
Regards,
--
Nélio Laranjeiro
6WIND
