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

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

On Fri, Aug 04, 2023 at 03:51:16PM +0800, Heng Qi wrote:
> 
> 
> å 2023/8/4 äå3:30, Parav Pandit åé:
> >>From: Heng Qi <hengqi@linux.alibaba.com>
> >>Sent: Friday, August 4, 2023 12:47 PM
> >>Yes, but a small update:
> >>If flowvq is created, RFF must be done only on flowvq by the driver.
> >>If flowvq is supported, but not created, cvq can be used.
> >>If flowvq is not supported, cvq is used.
> >>
> >Looks fine, I want to think little more on the last point to make sure we are not missing something.
> >Will respond by Monday on it.
> 
> Ok.
> 
> >[..]
> >
> >>Oh, I got it, I think we should let people see more details in the next version.:)
> >>
> >>>For any new/undefined protocol, if user wants to say, In a packet at
> >>>byte offset A, if you find pattern == 0x800, drop the packet.
> >>>In a packet at byte offset B, if you find pattern == 0x8100, forward to rq 10.
> >>>
> >>>I didn't consider multiple matching patterns for now, though it is very useful.
> >>>I am inclined to keep the option of _any_match to take up later, for now to do
> >>only well defined match?
> >>>WDYT?
> >>I think it's ok, but we don't seem to need the length here. But in any case, I
> >>don't think it's that important to have or not ^^
> >>
> >Length will be needed to indicate how many bytes to match to.
> >Lets do this rule incrementally after we get the base line done for known RFC defined fields.
> >
> >>>Lets keep rss context in future work as its orthogonal to it.
> >>>Yes, your help for rss context will be good.
> >>>Lets first finish RFF as its bit in the advance stage.
> >>Yes! But I worry that a spec that references a concept that doesn't exist in the
> >>existing spec may be blocked, so if you have no objections, I will push this work
> >>forward in the near future to help RFF possible blocking.
> >I was probably not clear enough, I propose that lets remove the RSS context for now in the RFF.
> >Once RSS context is done, at that point in future to enhance RFF.
Hi, Parav.

We need the RSS context, which can be combined with 'ethtool -X .. equal'
to achieve the purpose of traffic isolation, so please keep the
RSS context. To allay your concerns that an RSS context might be
blocking the work of n-tuples, I'll be issuing a spec this week (or next
week) for virtio support for the RSS context. For n-tuple RFF context,
we can support it after RFF, as a point to enhance RFF.

> >[..]

> +9. The filter rule add/delete entries are usually short in size of few tens of
> +   bytes, for example IPv6 + TCP tuple would be 36 bytes, and ops/sec rate is
> +   high, hence supplying fields inside the queue descriptor is preferred for
> +   up to a certain fixed size, say 56 bytes.

'56B' does not seem to be enough. For example,
src mac + dst mac + src-ip6 + dst-ip6 + src-port + dst-port + user-define
(packet byte offset pattern, length and mask) + flow id + rule priority + destination + action =
6 + 6 + 16 + 16 + 2 + 2 + 8 + 4 + 1 + 2 + 1 = 64 B,
we also have structure alignment and quintuple mask, etc.

> [..]
> +5. The device should be able to expose if it support filter masks.
> [..]
> +7. Flow filter capabilities to query using a DMA interface:
> +
> +```
> +struct flow_filter_capabilities {
> +	u8 flow_groups;
> +	u16 num_flow_filter_vqs;
> +	u16 start_vq_index;
> +	u32 max_flow_filters_per_group;
> +	u32 max_flow_filters;
> +	u64 supported_packet_field_mask_bmap[4];

I think the function here is that after the user sends the mask, the
driver should do an 'AND' operation with the mask supported by the device first?
Meanwhile, is 32B enough:) :
  src-port + dst-port + src mac + dst mac + src-ip6 + dst-ip6 =
    2 + 2 + 6 + 6 + 16 + 16 = 48B

Thanks!

> +};
> +
> +```