Charlie Somerville <char...@charlie.bz> writes:
On Mon, Jan 11, 2021, at 04:31, Shay Agroskin wrote:
Is this addition needed ? Seems like we don't set VIRTIO_XDP_TX
bit in case of virtnet_xdp_xmit() failure, so the surrounding
'if'
won't be taken.
Good catch, it looks like you're right. I'm happy to remove that
extra branch although I would like to add a comment explaining
that assumption:
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ed08998765e0..3ae7cd2f1e72 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1472,8 +1472,10 @@ static int virtnet_poll(struct
napi_struct *napi, int budget)
xdp_do_flush();
if (xdp_xmit & VIRTIO_XDP_TX) {
+ /* VIRTIO_XDP_TX only set on successful
virtnet_xdp_xmit,
+ * implies sq != NULL */
sq = virtnet_xdp_sq(vi);
- if (sq && virtqueue_kick_prepare(sq->vq) &&
virtqueue_notify(sq->vq)) {
+ if (virtqueue_kick_prepare(sq->vq) &&
virtqueue_notify(sq->vq)) {
u64_stats_update_begin(&sq->stats.syncp);
sq->stats.kicks++;
u64_stats_update_end(&sq->stats.syncp);
The comment itself looks good. Note that the code style dictates
that multi-line comments should end up with a line containing '*/'
alone.
See
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#commenting
for more information
Also you'd probably need to send a new email containing the new
patchset (see V# tag on emails in the mailing list)
Shay
