linux-kernel - Re: [RFC PATCH v3] can: virtio: Initial virtio CAN driver.

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: <3950ac4f-3292-e6ca-7484-77f3a5639305@opensynergy.com>
Date:   Sat, 13 May 2023 16:12:06 +0200
From:   Harald Mommer <harald.mommer@...nsynergy.com>
To:     Vincent MAILHOL <mailhol.vincent@...adoo.fr>,
        Mikhail Golubev-Ciuchea <Mikhail.Golubev-Ciuchea@...nsynergy.com>
Cc:     virtio-dev@...ts.oasis-open.org, linux-can@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Damir Shaikhutdinov <Damir.Shaikhutdinov@...nsynergy.com>
Subject: Re: [RFC PATCH v3] can: virtio: Initial virtio CAN driver.

Hello Vincent,

sometimes it's close to weekend and already late. I've to correct myself.

On 12.05.23 11:53, Vincent MAILHOL wrote:
>
>> +static netdev_tx_t virtio_can_start_xmit(struct sk_buff *skb,
>> +                                        struct net_device *dev)
>> +{
>> +       struct virtio_can_priv *priv = netdev_priv(dev);
>> +       struct canfd_frame *cf = (struct canfd_frame *)skb->data;
>> +       struct virtio_can_tx *can_tx_msg;
>> +       struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_TX];
>> +       struct scatterlist sg_out[1];
>> +       struct scatterlist sg_in[1];
>> +       struct scatterlist *sgs[2];
This 2 here.
>
>> +       /* Normal queue stop when no transmission slots are left */
>> +       if (atomic_read(&priv->tx_inflight) >= priv->can.echo_skb_max ||
>> +           vq->num_free == 0 || (vq->num_free < 2 &&
> Replace the Magic number 2 with a #define.

Is this 2 here.

Obviously with my previous answer I switched into panic mode thinking 
already about explaining indirect descriptors and all kind of virtio 
details and the expression in depth not realizing any more that 
something different was requested.

Appropriate answer:

/* CAN TX needs 2 descriptors: 1 device readable and 1 device writable */
#define CAN_TX_DESC (1 + 1)

Or something with ARRAY_SIZE(sgs) to get the number of elements in sgs 
keeping the first 2 above.

And then I'll have to think again whether I really want to keep 
sgs_in[1] and sgs_out[1] as arrays. Not now, now is weekend.

>> +           !virtio_has_feature(vq->vdev, VIRTIO_RING_F_INDIRECT_DESC))) {
>> +               netif_stop_queue(dev);
>> +               netdev_dbg(dev, "TX: Normal stop queue\n");
>> +       }
>> +
>> +       spin_unlock_irqrestore(&priv->tx_lock, flags);
>> +
>> +kick:
>> +       if (netif_queue_stopped(dev) || !netdev_xmit_more()) {
>> +               if (!virtqueue_kick(vq))
>> +                       netdev_err(dev, "%s(): Kick failed\n", __func__);
>> +       }
>> +
>> +       return xmit_ret;
>> +}
>>