From: Satananda Burla <sburla@marvell.com>
To: Parav Pandit <parav@nvidia.com>,
"virtio-comment@lists.oasis-open.org"
<virtio-comment@lists.oasis-open.org>,
"mst@redhat.com" <mst@redhat.com>,
"cohuck@redhat.com" <cohuck@redhat.com>
Cc: Shahaf Shuler <shahafs@nvidia.com>,
"si-wei.liu@oracle.com" <si-wei.liu@oracle.com>,
"xuanzhuo@linux.alibaba.com" <xuanzhuo@linux.alibaba.com>,
Heng Qi <hengqi@linux.alibaba.com>
Subject: [virtio-comment] RE: [EXT] [PATCH 6/7] virtio-net: Add flow filter requests
Date: Wed, 4 Oct 2023 19:38:28 +0000 [thread overview]
Message-ID: <CO6PR18MB4466CDE559C46ED7821BBB53C3CBA@CO6PR18MB4466.namprd18.prod.outlook.com> (raw)
In-Reply-To: <PH0PR12MB548173860F1BFC2282056F12DCC6A@PH0PR12MB5481.namprd12.prod.outlook.com>
Hi Parav,
> -----Original Message-----
> From: Parav Pandit <parav@nvidia.com>
> Sent: Sunday, October 1, 2023 3:58 AM
> To: Satananda Burla <sburla@marvell.com>; virtio-comment@lists.oasis-
> open.org; mst@redhat.com; cohuck@redhat.com
> Cc: Shahaf Shuler <shahafs@nvidia.com>; si-wei.liu@oracle.com;
> xuanzhuo@linux.alibaba.com; Heng Qi <hengqi@linux.alibaba.com>
> Subject: RE: [EXT] [PATCH 6/7] virtio-net: Add flow filter requests
>
>
>
> > From: Satananda Burla <sburla@marvell.com>
> > Sent: Sunday, October 1, 2023 3:13 PM
>
>
> > > +struct virtio_net_ff_match_fields {
> > > + le32 num_entries;
> > > + struct virtio_net_ff_match_field match_entries[]; };
> > It is better to fix the network protocol order here.(l2,l3,l4).
>
> It is recommended but I am not sure if we should enforce it.
>
> How about writing it as order SHOULD follow the packet field order..
> This will cover across L2, L3 and within a given header type too.
Fine with me.
>
> > > +
> > > +#define VIRTIO_NET_FF_DEST_RQ 0
> > > +
> > > +struct virtio_net_ff_dest {
> > > + u8 dest_type;
> > > + u8 reserved[3];
> > > + union {
> > > + le16 vq_index;
> > > + le32 reserved1;
> > > + };
> > > +};
> > > +
> > > +struct virtio_net_ff_action {
> > > + u8 action;
> > > + u8 reserved[7];
> > > +};
> > > +
> > > +#define VIRTIO_NET_FF_ACTION_DROP 0
> > > +#define VIRTIO_NET_FF_ACTION_FORWARD 1
> > > +
> > > +struct virtio_net_ff_add {
> > > + u8 op;
> > > + u8 priority; /* higher the value, higher priority */
> > > + u16 group_id;
> > > + le32 id;
> > > + struct virtio_net_ff_match_fields match;
> > > + struct virtio_net_ff_dest dest;
> > > + struct virtio_net_ff_action action;
> > Isn't dest also an action ? for instance, if ACTION is drop, there is no
> point in
> > adding destination, similarly if ACTION is forward, can we not add the
> vq_index
> > to the action itself using the reserved bytes? based on action type, the
> > additional bytes could be interpreted.
Any comment on this?
> > > +
> > > + struct virtio_net_ff_match_fields mask; /* optional */
> > How is the presence of this field indicated if it is optional ?
> > Does it need a field like has_mask?
> Yes. I missed it.
>
> > Can this be not added into struct virtio_net_ff_match_field ?
> I think yes, the issue is not all use cases (like ARFS) needs mask, who
> always uses exact values.
> So communicating and setting mask is redundant (also the struct is bit
> large).
>
> But I like your suggestion that we can have mask defined as, just
> match_value.
> Since value cannot live without its supporting type, finding it hard to
> have only value for the mask.
>
> > The mask only applies to struct virtio_net_ff_match_value. So it may be
> better
> > to just use struct virtio_net_ff_match_field {
> > struct virtio_net_ff_match_type type;
> > struct virtio_net_ff_match_value value;
> > struct virtio_net_ff_match_value mask;
> > };
> > instead of repeating the type again.
> > > +
> How about encoding has_mask in the type, so that mask can be optional in
> the above struct you proposed?
> With that we get both the benefits of size saving and computation too.
Yes. Makes sense.
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
next prev parent reply other threads:[~2023-10-04 19:38 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 13:48 [virtio-comment] [PATCH 0/7] virtio-net: Support flow filter for receive packets Parav Pandit
2023-09-22 13:48 ` [virtio-comment] [PATCH 1/7] virtio-net: Add theory of operation for flow filter Parav Pandit
2023-09-22 13:48 ` [virtio-comment] [PATCH 2/7] virtio-net: Add flow filter capabilities read commands Parav Pandit
2023-10-01 9:42 ` [virtio-comment] RE: [EXT] " Satananda Burla
2023-10-01 10:59 ` Parav Pandit
2023-09-22 13:48 ` [virtio-comment] [PATCH 3/7] virtio-net: Add flow filter group life cycle commands Parav Pandit
2023-09-22 13:48 ` [virtio-comment] [PATCH 4/7] virtio-net: Add flow filter transport set command Parav Pandit
2023-09-22 13:48 ` [virtio-comment] [PATCH 5/7] virtio-net: Add flow filter match values Parav Pandit
2023-09-22 13:48 ` [virtio-comment] [PATCH 6/7] virtio-net: Add flow filter requests Parav Pandit
2023-10-01 9:42 ` [virtio-comment] RE: [EXT] " Satananda Burla
2023-10-01 10:58 ` Parav Pandit
2023-10-04 19:38 ` Satananda Burla [this message]
2023-09-22 13:48 ` [virtio-comment] [PATCH 7/7] virtio-net: Add flow filter device and driver requirements Parav Pandit
2023-10-01 9:42 ` [virtio-comment] RE: [EXT] " Satananda Burla
2023-10-01 10:39 ` Parav Pandit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO6PR18MB4466CDE559C46ED7821BBB53C3CBA@CO6PR18MB4466.namprd18.prod.outlook.com \
--to=sburla@marvell.com \
--cc=cohuck@redhat.com \
--cc=hengqi@linux.alibaba.com \
--cc=mst@redhat.com \
--cc=parav@nvidia.com \
--cc=shahafs@nvidia.com \
--cc=si-wei.liu@oracle.com \
--cc=virtio-comment@lists.oasis-open.org \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).