On Fri, Apr 13, 2018 at 03:22:50PM +0000, Xueming(Steven) Li wrote:
>[...]
> > @@
> > > > static
> > > > > const struct mlx5_flow_items mlx5_flow_items[] = {
> > > > > .convert = mlx5_flow_create_vxlan_gpe,
> > > > > .dst_sz = sizeof(struct ibv_flow_spec_tunnel),
> > > > > },
> > > > > + [RTE_FLOW_ITEM_TYPE_MPLS] = {
> > > > > + .items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > > > > + RTE_FLOW_ITEM_TYPE_IPV4,
> > > > > + RTE_FLOW_ITEM_TYPE_IPV6),
> > > > > + .actions = valid_actions,
> > > > > + .mask = &(const struct rte_flow_item_mpls){
> > > > > + .label_tc_s = "\xff\xff\xf0",
> > > > > + },
> > > > > + .default_mask = &rte_flow_item_mpls_mask,
> > > > > + .mask_sz = sizeof(struct rte_flow_item_mpls),
> > > > > + .convert = mlx5_flow_create_mpls, #ifdef
> > > > > +HAVE_IBV_DEVICE_MPLS_SUPPORT
> > > > > + .dst_sz = sizeof(struct ibv_flow_spec_mpls), #endif
> > > > > + },
> > > >
> > > > Why the whole item is not under ifdef?
> > >
> > > If apply macro to whole item, there will be a null pointer if create
> > mpls flow.
> > > There is a macro in function mlx5_flow_create_mpls() to avoid using this
> > invalid data.
> >
> > I think there is some kind of confusion here, what I mean is moving the
> > #ifdef to embrace the whole stuff i.e.:
> >
> > #ifdef HAVE_IBV_DEVICE_MPLS_SUPPORT
> > [RTE_FLOW_ITEM_TYPE_MPLS] = {
> > .items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > RTE_FLOW_ITEM_TYPE_IPV4,
> > RTE_FLOW_ITEM_TYPE_IPV6),
> > .actions = valid_actions,
> > .mask = &(const struct rte_flow_item_mpls){
> > .label_tc_s = "\xff\xff\xf0",
> > },
> > .default_mask = &rte_flow_item_mpls_mask,
> > .mask_sz = sizeof(struct rte_flow_item_mpls),
> > .convert = mlx5_flow_create_mpls,
> > .dst_sz = sizeof(struct ibv_flow_spec_mpls) #endif
> >
> > Not having this item in this static array ends by not supporting it, this
> > is what I mean.
>
> Yes, I know. There is a code using this array w/o NULL check:
> cur_item = &mlx5_flow_items[items->type];
> ret = cur_item->convert(items,
> (cur_item->default_mask ?
> cur_item->default_mask :
> cur_item->mask),
> &data);
>
>
This code is after the mlx5_flow_convert_items_validate() which refuses
unknown items, if you you see an unknown item reaching this code above,
there is bug somewhere and it should be fixed. Un-supported items
should not be in the static array.
Regards,
--
Nélio Laranjeiro
6WIND
