From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alvaro Karsz <alvaro.karsz@solid-run.com>
Cc: netdev@vger.kernel.org, Jason Wang <jasowang@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Rabeeh Khoury <rabeeh@solid-run.com>
Subject: Re: [PATCH] net: virtio_net: support interrupt coalescing
Date: Thu, 12 May 2022 09:41:44 -0400 [thread overview]
Message-ID: <20220512094112-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CAJs=3_CnyCLBeW8kz49Z+VzGNpgrViMp+nngg_CWUuq9s1Oyiw@mail.gmail.com>
On Thu, May 12, 2022 at 03:54:28PM +0300, Alvaro Karsz wrote:
> So first, please at least reserve the feature in the virtio spec.
> Preferably add the specification as well.
> Here's one way to do it:
> https://github.com/oasis-tcs/virtio-spec#use-of-github-issues
>
>
> Sorry about that.
> I sent a patch to the virtio comment list, but thought that I should wait
> until I got a few comments before opening a github issue.
> https://lists.oasis-open.org/archives/virtio-comment/202205/msg00016.html
> Should I open the github issue now? or should I wait a little for more comments
> on the virtio-spec patch?
At this point you can wait.
>
> You want all these to be LE. Native endian's unpredictable in lots of
> virt settings.
>
>
> I will fix this, wait a few more days to see if there are more comments, and
> then create a new version of the patch.
>
> Thanks
>
> On Thu, May 12, 2022 at 12:55 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, May 12, 2022 at 11:35:23AM +0300, Alvaro Karsz wrote:
> > Control a Virtio network device interrupt coalescing parameters
> > using the control virtqueue.
> >
> > New VirtIO network feature: VIRTIO_NET_F_INTR_COAL.
> >
> > A device that supports this fetature can receive
> > VIRTIO_NET_CTRL_INTR_COAL control commands.
> >
> > * VIRTIO_NET_CTRL_INTR_COAL_USECS_SET:
> > change the rx-usecs and tx-usecs parameters.
> >
> > rx-usecs - Time to delay an RX interrupt after packet arrival in
> > microseconds.
> >
> > tx-usecs - Time to delay a TX interrupt after a sending a packet
> > in microseconds.
> >
> > * VIRTIO_NET_CTRL_INTR_COAL_FRAMES_SET:
> > change the rx-max-frames and tx-max-frames parameters.
> >
> > rx-max-frames: Number of packets to delay an RX interrupt after
> > packet arrival.
> >
> > tx-max-frames: Number of packets to delay a TX interrupt after
> > sending a packet.
> >
> > Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
>
> So first, please at least reserve the feature in the virtio spec.
> Preferably add the specification as well.
> Here's one way to do it:
> https://github.com/oasis-tcs/virtio-spec#use-of-github-issues
>
>
> > ---
> > drivers/net/virtio_net.c | 108 ++++++++++++++++++++++++++++----
> > include/uapi/linux/virtio_net.h | 34 +++++++++-
> > 2 files changed, 128 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index cbba9d2e8f3..4806c35ddd5 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -261,6 +261,12 @@ struct virtnet_info {
> > u8 duplex;
> > u32 speed;
> >
> > + /* Interrupt coalescing settings */
> > + u32 tx_usecs;
> > + u32 rx_usecs;
> > + u32 tx_frames_max;
> > + u32 rx_frames_max;
> > +
> > unsigned long guest_offloads;
> > unsigned long guest_offloads_capable;
> >
> > @@ -2594,19 +2600,75 @@ static int virtnet_set_coalesce(struct net_device
> *dev,
> > {
> > struct virtnet_info *vi = netdev_priv(dev);
> > int i, napi_weight;
> > + struct scatterlist sgs_usecs, sgs_frames;
> > + struct virtio_net_ctrl_coal_frames c_frames;
> > + struct virtio_net_ctrl_coal_usec c_usecs;
> > + bool update_napi,
> > + intr_coal = virtio_has_feature(vi->vdev, VIRTIO_NET_F_INTR_COAL);
> > +
> > + /* rx_coalesce_usecs/tx_coalesce_usecs are supported only
> > + * if VIRTIO_NET_F_INTR_COAL feature is set.
> > + */
> > + if (!intr_coal && (ec->rx_coalesce_usecs || ec->tx_coalesce_usecs))
> > + return -EOPNOTSUPP;
> > +
> > + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_INTR_COAL)) {
> > + /* Send usec command */
> > + c_usecs.tx_usecs = ec->tx_coalesce_usecs;
> > + c_usecs.rx_usecs = ec->rx_coalesce_usecs;
> > + sg_init_one(&sgs_usecs, &c_usecs, sizeof(c_usecs));
> > +
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_INTR_COAL,
> > +
> VIRTIO_NET_CTRL_INTR_COAL_USECS_SET,
> > + &sgs_usecs))
> > + return -EINVAL;
> > +
> > + /* Save parameters */
> > + vi->tx_usecs = ec->tx_coalesce_usecs;
> > + vi->rx_usecs = ec->rx_coalesce_usecs;
> > +
> > + /* Send frames command */
> > + c_frames.tx_frames_max = ec->tx_max_coalesced_frames;
> > + c_frames.rx_frames_max = ec->rx_max_coalesced_frames;
> > + sg_init_one(&sgs_frames, &c_frames, sizeof(c_frames));
> > +
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_INTR_COAL,
> > +
> VIRTIO_NET_CTRL_INTR_COAL_FRAMES_SET,
> > + &sgs_frames))
> > + return -EINVAL;
> > +
> > + /* Save parameters */
> > + vi->tx_frames_max = ec->tx_max_coalesced_frames;
> > + vi->rx_frames_max = ec->rx_max_coalesced_frames;
>
>
>
>
> > + }
> > +
> > + /* Should we update NAPI? */
> > + update_napi = ec->tx_max_coalesced_frames <= 1 &&
> > + ec->rx_max_coalesced_frames == 1;
> >
> > - if (ec->tx_max_coalesced_frames > 1 ||
> > - ec->rx_max_coalesced_frames != 1)
> > + /* If interrupt coalesing feature is not set, and we can't update
> NAPI, return an error */
> > + if (!intr_coal && !update_napi)
> > return -EINVAL;
> >
> > - napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> > - if (napi_weight ^ vi->sq[0].napi.weight) {
> > - if (dev->flags & IFF_UP)
> > - return -EBUSY;
> > - for (i = 0; i < vi->max_queue_pairs; i++)
> > - vi->sq[i].napi.weight = napi_weight;
> > + if (update_napi) {
> > + napi_weight = ec->tx_max_coalesced_frames ?
> NAPI_POLL_WEIGHT : 0;
> > + if (napi_weight ^ vi->sq[0].napi.weight) {
> > + if (dev->flags & IFF_UP) {
> > + /* If Interrupt coalescing feature is not
> set, return an error.
> > + * Otherwise exit without changing the NAPI
> paremeters
> > + */
> > + if (!intr_coal)
> > + return -EBUSY;
> > +
> > + goto exit;
> > + }
> > +
> > + for (i = 0; i < vi->max_queue_pairs; i++)
> > + vi->sq[i].napi.weight = napi_weight;
> > + }
> > }
> >
> > +exit:
> > return 0;
> > }
> >
> > @@ -2616,14 +2678,25 @@ static int virtnet_get_coalesce(struct net_device
> *dev,
> > struct netlink_ext_ack *extack)
> > {
> > struct ethtool_coalesce ec_default = {
> > - .cmd = ETHTOOL_GCOALESCE,
> > - .rx_max_coalesced_frames = 1,
> > + .cmd = ETHTOOL_GCOALESCE
> > };
> > +
> > struct virtnet_info *vi = netdev_priv(dev);
> > + bool intr_coal = virtio_has_feature(vi->vdev,
> VIRTIO_NET_F_INTR_COAL);
> > +
> > + /* Add Interrupt coalescing settings */
> > + if (intr_coal) {
> > + ec_default.rx_coalesce_usecs = vi->rx_usecs;
> > + ec_default.tx_coalesce_usecs = vi->tx_usecs;
> > + ec_default.tx_max_coalesced_frames = vi->tx_frames_max;
> > + ec_default.rx_max_coalesced_frames = vi->rx_frames_max;
> > + } else {
> > + ec_default.rx_max_coalesced_frames = 1;
> > + }
> >
> > memcpy(ec, &ec_default, sizeof(ec_default));
> >
> > - if (vi->sq[0].napi.weight)
> > + if (!intr_coal && vi->sq[0].napi.weight)
> > ec->tx_max_coalesced_frames = 1;
> >
> > return 0;
> > @@ -2743,7 +2816,7 @@ static int virtnet_set_rxnfc(struct net_device
> *dev, struct ethtool_rxnfc *info)
> > }
> >
> > static const struct ethtool_ops virtnet_ethtool_ops = {
> > - .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
> > + .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES |
> ETHTOOL_COALESCE_USECS,
> > .get_drvinfo = virtnet_get_drvinfo,
> > .get_link = ethtool_op_get_link,
> > .get_ringparam = virtnet_get_ringparam,
> > @@ -3423,6 +3496,8 @@ static bool virtnet_validate_features(struct
> virtio_device *vdev)
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
> > "VIRTIO_NET_F_CTRL_VQ") ||
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > + "VIRTIO_NET_F_CTRL_VQ") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_INTR_COAL,
> > "VIRTIO_NET_F_CTRL_VQ"))) {
> > return false;
> > }
> > @@ -3558,6 +3633,13 @@ static int virtnet_probe(struct virtio_device
> *vdev)
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> > vi->mergeable_rx_bufs = true;
> >
> > + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_INTR_COAL)) {
> > + vi->rx_usecs = 0;
> > + vi->tx_usecs = 0;
> > + vi->tx_frames_max = 0;
> > + vi->rx_frames_max = 0;
> > + }
> > +
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> > vi->has_rss_hash_report = true;
> >
> > @@ -3786,7 +3868,7 @@ static struct virtio_device_id id_table[] = {
> > VIRTIO_NET_F_CTRL_MAC_ADDR, \
> > VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> > - VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
> > + VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT, VIRTIO_NET_F_INTR_COAL
> >
> > static unsigned int features[] = {
> > VIRTNET_FEATURES,
> > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/
> virtio_net.h
> > index 3f55a4215f1..b65a4295270 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -56,7 +56,7 @@
> > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
> > * Steering */
> > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> > -
> > +#define VIRTIO_NET_F_INTR_COAL 55 /* Guest can handle
> Interrupt coalescing */
> > #define VIRTIO_NET_F_HASH_REPORT 57 /* Supports hash report */
> > #define VIRTIO_NET_F_RSS 60 /* Supports RSS RX steering */
> > #define VIRTIO_NET_F_RSC_EXT 61 /* extended coalescing info */
> > @@ -355,4 +355,36 @@ struct virtio_net_hash_config {
> > #define VIRTIO_NET_CTRL_GUEST_OFFLOADS 5
> > #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0
> >
> > +/*
> > + * Control interrupt coalescing.
> > + *
> > + * Request the device to change the interrupt coalescing parameters.
> > + *
> > + * Available with the VIRTIO_NET_F_INTR_COAL feature bit.
> > + */
> > +#define VIRTIO_NET_CTRL_INTR_COAL 6
> > +/*
> > + * Set the rx-usecs/tx-usecs patameters.
> > + * rx-usecs - Number of microseconds to delay an RX interrupt after
> packet arrival.
> > + * tx-usecs - Number of microseconds to delay a TX interrupt after a
> sending a packet.
> > + */
> > +struct virtio_net_ctrl_coal_usec {
> > + __u32 tx_usecs;
> > + __u32 rx_usecs;
> > +};
> > +
> > +#define VIRTIO_NET_CTRL_INTR_COAL_USECS_SET 0
> > +
> > +/*
> > + * Set the rx-max-frames/tx-max-frames patameters.
> > + * rx-max-frames - Number of packets to delay an RX interrupt after
> packet arrival.
> > + * tx-max-frames - Number of packets to delay a TX interrupt after
> sending a packet.
> > + */
> > +struct virtio_net_ctrl_coal_frames {
> > + __u32 tx_frames_max;
> > + __u32 rx_frames_max;
> > +};
> > +
>
> You want all these to be LE. Native endian's unpredictable in lots of
> virt settings.
>
>
> > +#define VIRTIO_NET_CTRL_INTR_COAL_FRAMES_SET 1
> > +
> > #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> > --
> > 2.32.0
>
>
>
>
> --
> [uc]
>
> Alvaro Karsz, Software
> +972-50-7696862| https://www.solid-run.com
prev parent reply other threads:[~2022-05-12 13:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-12 8:35 [PATCH] net: virtio_net: support interrupt coalescing Alvaro Karsz
2022-05-12 9:55 ` Michael S. Tsirkin
[not found] ` <CAJs=3_CnyCLBeW8kz49Z+VzGNpgrViMp+nngg_CWUuq9s1Oyiw@mail.gmail.com>
2022-05-12 13:41 ` Michael S. Tsirkin [this message]
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=20220512094112-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=alvaro.karsz@solid-run.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jasowang@redhat.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=rabeeh@solid-run.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.