Re: [PATCH requirements v4 5/7] net-features: Add n-tuple receive flow filters requirements

[Heng Qi <hengqi@linux.alibaba.com>]

å 2023/8/17 äå12:52, Parav Pandit åé:
> > From: Heng Qi <hengqi@linux.alibaba.com>
> > Sent: Wednesday, August 16, 2023 5:12 PM
> > > +/* command 1 */
> > > +struct flow_filter_capabilities {
> > > +	le16 start_vq_index;
> > > +	le16 num_flow_filter_vqs;
> > > +	le16 max_flow_groups;
> > > +	le16 max_group_priorities; /* max priorities of the group */
> > > +	le32 max_flow_filters_per_group;
> > > +	le32 max_flow_filters; /* max flow_id in add/del
> > > +				* is equal = max_flow_filters - 1.
> > > +				*/
> > > +	u8 max_priorities_per_group;
> > + u8 padding[3];
> Ack.
>
> > > +struct virtio_net_rff_group_add {
> > > +	le16 priority;
> > Please explicitly explain the relationship between the number and the priority,
> > for example, the smaller the number, the higher the priority :)
> Right. Will do.
> I was thinking of higher the value higher the priority, so that one doesnt need to invert this in brain every time seeing the priority field. :)
>   

It's ok :)

> > > +	le16 group_id;
> > > +};
> > > +
> > > +
> > > +struct virtio_net_rff_group_delete {
> > > +	le16 group_id;
> > > +
> > > +```
> > > +
> > > +3. Flow filter entry add/modify, delete over flow vq:
> > > +
> > > +```
> > > +struct virtio_net_rff_add_modify {
> > > +	u8 flow_op;
> > > +	u8 padding;
> > s/padding/priority
> Ack.
>
> > Each rule needs a priority.
> >
> > > +	u16 group_id;
> > > +	le32 flow_id;
> > > +	struct match_criteria mc;
> > > +	struct destination dest;
> > > +	struct action action;
> > > +
> > > +	struct match_criteria mask;	/* optional */
> > > +};
> > > +
> > > +struct virtio_net_rff_delete {
> > > +	u8 flow_op;
> > > +	u8 padding[3];
> > > +	le32 flow_id;
> > > +};
> > > +
> > > +```
> > > +
> > > +4. Flow filter commands over cvq:
> > > +
> > > +```
> > > +
> > > +struct virtio_net_rff_cmd {
> > > +	u8 class; /* RFF class */
> > > +	u8 commands; /* RFF cmd = A */
> > > +	u8 command-specific-data[]; /* contains struct
> > virtio_net_rff_add_modify or
> > > +				     * struct virtio_net_rff_delete
> > For flow vq, we no longer distinguish operations by command, but by flow_op.
> > But for ctrlq, this field will be carried. We should make it clear that when ctrlq is
> > delivered based on cmd, the flow_op field is ignored.
> Since cvq is only the communication medium for delivering of command, it is better use the flow_op as is, and cvq commands field to be ignored.
> This way, software layers are more organized cvq or flow vq.

Agree.

Thanks!

>