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

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

å 2023/6/13 äå8:24, Parav Pandit åé:
> > From: Heng Qi <hengqi@linux.alibaba.com>
> > Sent: Tuesday, June 13, 2023 1:04 AM
> >
> >
> > å 2023/6/13 äå12:16, Parav Pandit åé:
> > > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > > Sent: Monday, June 12, 2023 10:57 PM
> > > [..]
> > > > > +4. The match criteria may optionally also include specific packet
> > > > > +data offset,
> > > > Is this the offset of the packet payload (not including the packet
> > > > header)? If yes, what might be the usage scenarios?
> > > Some packet header fields may not be defined initially by us.
> > > For example some protocol FOO type which has some header after UDP
> > header.
> > > And wants to filter and steer to a specific RQ.
> > Ok, I got it.
> >
> > > A virtio may not have defined such protocol at the beginning. So a generic
> > extension allows to steer packet.
> > > At that point line between header and data is blurry. :)
> > Then I think it's just an *offset* from the start of the packet so we can use this
> > capability for the packet header as well.
> Yes, will change it. "packet data" was misleading. It is just the packet offset.
>   
> > > > > +   length, and matching pattern, which may not be defined in the
> > > > > + standard
> > > > RFC.
> > > > > +5. Action includes (a) dropping or (b) forwarding the packet.
> > > > > +6. Destination location is a receive virtqueue index or rss context.
> > > > > +7. The device should process RFS rules before RSS rules, i.e., when there is
> > a
> > > > > +   miss on the RFS rule RSS rule applies.
> > > > > +8. The device should be able to add/remove these rules at a rate of 100K
> > > > > +   rules/sec.
> > > > Why do we need to add this limit for the device please? I think that
> > > > different devices may handle ctrq at different speeds.
> > > ARFS rules insertion is fast and tcp streams should be able to converge quicker
> > on the desired cpu.
> > > So rule insertion should be fast enough.
> > > Many of the device can/may be able to do this as some semi-fast path
> > operation unlike CVQ which is typically a device level control operation, often
> > done as slow process.
> >
> > We need to consider ARFS for receive flow filters, but receive flow filters are
> > more infrastructure, do we need to offer a sub-feature to distinguish whether a
> > device supports this dedicated queue or not.
> > This becomes easier for users who only use receive flow filters without ARFS
> > enabled.
> > At this point, ARFS and receive flow filters become orthogonal.
> If itâs a "flowvq" dedicated queue device both the requirements are addressed.
> Device can choose to implement in its preferred way.

I think this is a migration issue:
Devices A and B each offser the VIRTIO_NET_F_RECEIVE_FLOW_FILTER (this 
is a temporary name) feature.
And device A implements flowvq, device B does not implement flowvq, they 
are both using receive flow filters but ARFS is disabled.
The migration should fail at this time. Does this meet user expectations?

Or do we warn in the spec that using flow vq might cause the migration 
issue?

Thanks!

> > > So driver-device interface to be able to scale up to low latency and high
> > throughput flow insert/delete ops.
> >
> > Ok.
> >
> > > It is ok one device may choose to not support such high rate, but interface
> > definition wise I imagine to be on its own dedicated queue.
> > > > > +9. If multiple rules are programmed which has overlapping attributes for a
> > > > > +   received packet, the driver to define the location/priority of the rule.
> > > > > +10. The driver should be able to query flow steering table entries
> > > > programmed in
> > > > > +    the device by the flow id.
> > > > Combining points 9 and 10, can I deduce that the attributes of a flow
> > > > rule include identifier (id), position (index of the rule entry) and priority?
> > > Yes, I also think that way.
> > >
> > > > According to point 11, the driver needs to provide a unique id, if
> > > > the location and priority are not provided, they should have predefined
> > default values.
> > > Yes.
> > > I am not fully sure of it as it brings some uncertainty in behavior.
> > > So having second thought as driver to always provide it.
> > Yes, and this point is specific, then we can continue to confirm it in the future
> > spec version.
> Ok. I will revise it in v1.
>
> > > > > +11. The driver should be able to add the entry for attributes (a) match
> > > > > +    criteria, (b) action, (c) destination and (d) assign a unique id of 32 bits.
> > > > > +12. The driver should be able to delete the steering rule entry via
> > > > > +a unique
> > > > id.
> > > > > +13. The driver should be able to add and delete the steering rules in out of
> > > > > +    order manner without depending on previous commands.
> > > > > +14. A group member device should be able to query the attributes of
> > > > > +the
> > > > flow
> > > > > +    steering that device supports.
> > > > Does a group member driver support insertion rules? The group owner
> > > > driver and a group member driver should have the ability to
> > > > add/remove rules for themselves.
> > > Yes, Group owner adds rule for the self, not on behalf.
> > > Same as group member.
> > > No difference between them as they operate on self device.
> > Ok.
> >
> > > In future, a group owner may want to steer the packet towards a group
> > member, like eswitch.
> >
> > Yes.
> > And what I want to say is that our cloud architecture is similar to two-layer
> > virtualization, but we only have one layer of eswitch, so we need VF id to let
> > eswitch provide simple flow steering for the second layer of virtualization, and
> > do not need complicated eswitch.
> > This is the purpose of carrying VF id, but now we can not consider this (VF id) in
> > the feature of receive flow filters, so as not to hinder the progress of this
> > feature.
> Ok. Lets keep this in notes and mind to have the abstraction and donât jump to bake vf id in the spec in the initial version.