On Thu, Jul 05, 2018 at 12:56:09PM -0700, Yongseok Koh wrote: >[...] > > > > + if (mark->id >= MLX5_FLOW_MARK_MAX) > > > > + return rte_flow_error_set(error, EINVAL, > > > > + > > > > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > > > > + &mark->id, > > > > + "mark must be between 0 and" > > > > + " 16777199"); > > > > > > Use %d and (MLX5_FLOW_MARK_MAX - 1), instead of fixed string. > > > > It needs an snprintf, rte_flow_error_set() does not accept formatting > > strings. > > I think the following would work but never mind. I'm okay with leaving it as > is. > No need to make a change here. > > #define STRINGIFY(x) #x > #define TOSTRING(x) STRINGIFY(x) > "mark must be between 0 and " > TOSTRING(MLX5_FLOW_MARK_MAX - 1)); >
It was to avoid adding a macro, but indeed there is the same kind in mlx4, I'll port them for mlx5. > > >[...] > > Addressing both question here, for the flow_stop() and flow_destroy() > > the process is different, for the stop, the flow remains with the mark > > bit set but all queues must me cleared, there is no comparison to make. > > As you can see, it don't even get a flow, it directly unset the mask bit > > in the Rx queues. > > For the destroy the issue is different, several flows may be using the > > same Rx queues, if one of them will remains and has a mark, then the > > associated queues must keep their mark bit set. > > As the process is different, it would end in two distinct functions and > > each one used by a single function. > > > > For the mlx5_flow_rxq_mark(), the situation is different, the same > > process is make when a flow is created and the flow are started. > > I knew the differences but I just wanted to ask if having a separate function > can be a viable option, e.g., > > mlx5_flow_rxq_mark_set() > mlx5_flow_rxq_mark_clear() > mlx5_flow_rxq_mark_trim() Certainly, the point is those functions have a short life as few patches letter they will be removed. I suppose you prefer to have them and I don't think it will take too much time to add such function, it will make part of the next revision ;). Thanks, -- Nélio Laranjeiro 6WIND
