lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA93jw6Z2vfh3cAVbmnHTsvbfNoqhdjdfAjrbKDyCeV9wHHv7w@mail.gmail.com>
Date:   Sun, 17 Jul 2022 07:09:38 -0700
From:   Dave Taht <dave.taht@...il.com>
To:     Alvaro Karsz <alvaro.karsz@...id-run.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [RFC PATCH net-next v3] net: virtio_net: notifications coalescing support

On Thu, Jul 14, 2022 at 7:29 AM Alvaro Karsz <alvaro.karsz@...id-run.com> wrote:
>
> New VirtIO network feature: VIRTIO_NET_F_NOTF_COAL.
>
> Control a Virtio network device notifications coalescing parameters
> using the control virtqueue.

What are a typical range of settings for these?


> A device that supports this fetature can receive
> VIRTIO_NET_CTRL_NOTF_COAL control commands.
>
> - VIRTIO_NET_CTRL_NOTF_COAL_TX_SET:
>   Ask the network device to change the following parameters:
>   - tx_usecs: Maximum number of usecs to delay a TX notification.
>   - tx_max_packets: Maximum number of packets to send before a
>     TX notification.
>
> - VIRTIO_NET_CTRL_NOTF_COAL_RX_SET:
>   Ask the network device to change the following parameters:
>   - rx_usecs: Maximum number of usecs to delay a RX notification.
>   - rx_max_packets: Maximum number of packets to receive before a
>     RX notification.

Bytes = time.  Packets nowadays have a dynamic range of 64-64k bytes,
and with big TCP even more. would there be any way to use
bytes rather than packets?

https://lwn.net/Articles/469652/
>
> VirtIO spec. patch:
> https://lists.oasis-open.org/archives/virtio-comment/202206/msg00100.html
>
> Signed-off-by: Alvaro Karsz <alvaro.karsz@...id-run.com>
> ---
> v2:
>         - Fix type assignments warnings found with sparse.
>         - Fix a few typos.
>
> v3:
>   - Change the coalescing parameters in a dedicated function.
>   - Return -EBUSY from the set coalescing function when the device's
>     link is up, even if the notifications coalescing feature is negotiated.
>
> ---
>  drivers/net/virtio_net.c        | 127 +++++++++++++++++++++++++++-----
>  include/uapi/linux/virtio_net.h |  34 ++++++++-
>  2 files changed, 140 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 356cf8dd416..00905f2e2f2 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_max_packets;
> +       u32 rx_max_packets;
> +
>         unsigned long guest_offloads;
>         unsigned long guest_offloads_capable;
>
> @@ -2587,44 +2593,115 @@ static int virtnet_get_link_ksettings(struct net_device *dev,
>         return 0;
>  }
>
> -static int virtnet_set_coalesce(struct net_device *dev,
> -                               struct ethtool_coalesce *ec,
> -                               struct kernel_ethtool_coalesce *kernel_coal,
> -                               struct netlink_ext_ack *extack)
> +static int virtnet_update_napi_weight(struct net_device *dev,
> +                                     int napi_weight)
>  {
>         struct virtnet_info *vi = netdev_priv(dev);
> -       int i, napi_weight;
> -
> -       if (ec->tx_max_coalesced_frames > 1 ||
> -           ec->rx_max_coalesced_frames != 1)
> -               return -EINVAL;
> +       int i;
>
> -       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;
>         }
> -
>         return 0;
>  }
>
> +static int virtnet_set_notf_coal(struct net_device *dev,
> +                                struct ethtool_coalesce *ec)
> +{
> +       struct virtnet_info *vi = netdev_priv(dev);
> +       struct scatterlist sgs_tx, sgs_rx;
> +       struct virtio_net_ctrl_coal_tx coal_tx;
> +       struct virtio_net_ctrl_coal_rx coal_rx;
> +       int ret, napi_weight;
> +
> +       coal_tx.tx_usecs = cpu_to_le32(ec->tx_coalesce_usecs);
> +       coal_tx.tx_max_packets = cpu_to_le32(ec->tx_max_coalesced_frames);
> +       sg_init_one(&sgs_tx, &coal_tx, sizeof(coal_tx));
> +
> +       if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> +                                 VIRTIO_NET_CTRL_NOTF_COAL_TX_SET,
> +                                 &sgs_tx))
> +               return -EINVAL;
> +
> +       /* Save parameters */
> +       vi->tx_usecs = ec->tx_coalesce_usecs;
> +       vi->tx_max_packets = ec->tx_max_coalesced_frames;
> +
> +       coal_rx.rx_usecs = cpu_to_le32(ec->rx_coalesce_usecs);
> +       coal_rx.rx_max_packets = cpu_to_le32(ec->rx_max_coalesced_frames);
> +       sg_init_one(&sgs_rx, &coal_rx, sizeof(coal_rx));
> +
> +       if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> +                                 VIRTIO_NET_CTRL_NOTF_COAL_RX_SET,
> +                                 &sgs_rx))
> +               return -EINVAL;
> +
> +       /* Save parameters */
> +       vi->rx_usecs = ec->rx_coalesce_usecs;
> +       vi->rx_max_packets = ec->rx_max_coalesced_frames;
> +
> +       napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> +       ret = virtnet_update_napi_weight(dev, napi_weight);
> +       return ret;
> +}
> +
> +static int virtnet_set_notf_coal_napi(struct net_device *dev,
> +                                     struct ethtool_coalesce *ec)
> +{
> +       int ret, napi_weight;
> +
> +       /* usecs coalescing is supported only if VIRTIO_NET_F_NOTF_COAL
> +        * feature is negotiated.
> +        */
> +       if (ec->rx_coalesce_usecs || ec->tx_coalesce_usecs)
> +               return -EOPNOTSUPP;
> +
> +       if (ec->tx_max_coalesced_frames > 1 ||
> +           ec->rx_max_coalesced_frames != 1)
> +               return -EINVAL;
> +
> +       napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> +       ret = virtnet_update_napi_weight(dev, napi_weight);
> +       return ret;
> +}
> +
> +static int virtnet_set_coalesce(struct net_device *dev,
> +                               struct ethtool_coalesce *ec,
> +                               struct kernel_ethtool_coalesce *kernel_coal,
> +                               struct netlink_ext_ack *extack)
> +{
> +       struct virtnet_info *vi = netdev_priv(dev);
> +       int ret;
> +
> +       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL))
> +               ret = virtnet_set_notf_coal(dev, ec);
> +       else
> +               ret = virtnet_set_notf_coal_napi(dev, ec);
> +
> +       return ret;
> +}
> +
>  static int virtnet_get_coalesce(struct net_device *dev,
>                                 struct ethtool_coalesce *ec,
>                                 struct kernel_ethtool_coalesce *kernel_coal,
>                                 struct netlink_ext_ack *extack)
>  {
> -       struct ethtool_coalesce ec_default = {
> -               .cmd = ETHTOOL_GCOALESCE,
> -               .rx_max_coalesced_frames = 1,
> -       };
>         struct virtnet_info *vi = netdev_priv(dev);
>
> -       memcpy(ec, &ec_default, sizeof(ec_default));
> +       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> +               ec->rx_coalesce_usecs = vi->rx_usecs;
> +               ec->tx_coalesce_usecs = vi->tx_usecs;
> +               ec->tx_max_coalesced_frames = vi->tx_max_packets;
> +               ec->rx_max_coalesced_frames = vi->rx_max_packets;
> +       } else {
> +               ec->rx_max_coalesced_frames = 1;
>
> -       if (vi->sq[0].napi.weight)
> -               ec->tx_max_coalesced_frames = 1;
> +               if (vi->sq[0].napi.weight)
> +                       ec->tx_max_coalesced_frames = 1;
> +       }
>
>         return 0;
>  }
> @@ -2743,7 +2820,8 @@ 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,
> @@ -3411,6 +3489,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_NOTF_COAL,
>                              "VIRTIO_NET_F_CTRL_VQ"))) {
>                 return false;
>         }
> @@ -3546,6 +3626,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_NOTF_COAL)) {
> +               vi->rx_usecs = 0;
> +               vi->tx_usecs = 0;
> +               vi->tx_max_packets = 0;
> +               vi->rx_max_packets = 0;
> +       }
> +
>         if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
>                 vi->has_rss_hash_report = true;
>
> @@ -3780,7 +3867,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_NOTF_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..29ced55514d 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_NOTF_COAL 53      /* Guest can handle notifications 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 notifications coalescing.
> + *
> + * Request the device to change the notifications coalescing parameters.
> + *
> + * Available with the VIRTIO_NET_F_NOTF_COAL feature bit.
> + */
> +#define VIRTIO_NET_CTRL_NOTF_COAL              6
> +/*
> + * Set the tx-usecs/tx-max-packets patameters.
> + * tx-usecs - Maximum number of usecs to delay a TX notification.
> + * tx-max-packets - Maximum number of packets to send before a TX notification.
> + */
> +struct virtio_net_ctrl_coal_tx {
> +       __le32 tx_max_packets;
> +       __le32 tx_usecs;
> +};
> +
> +#define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET               0
> +
> +/*
> + * Set the rx-usecs/rx-max-packets patameters.
> + * rx-usecs - Maximum number of usecs to delay a RX notification.
> + * rx-max-frames - Maximum number of packets to receive before a RX notification.
> + */
> +struct virtio_net_ctrl_coal_rx {
> +       __le32 rx_max_packets;
> +       __le32 rx_usecs;
> +};
> +
> +#define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET               1
> +
>  #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> --
> 2.32.0



-- 
FQ World Domination pending: https://blog.cerowrt.org/post/state_of_fq_codel/
Dave Täht CEO, TekLibre, LLC

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ