* [PATCH net-next 1/3] virtio_net: set multicast filter list to host
@ 2013-12-10 0:16 Stephen Hemminger
2013-12-10 0:17 ` [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command Stephen Hemminger
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Stephen Hemminger @ 2013-12-10 0:16 UTC (permalink / raw)
To: David Miller, Rusty Russell, Michael S. Tsirkin; +Cc: virtualization, netdev
The virtio_net driver never sends the multicast address list to
the host. This is because send command takes a pointer to scatter list
to send but only inserts that one entry into the outgoing scatter list.
This bug has been there since:
commit f565a7c259d71cc186753653d978c646d2354b36
Author: Alex Williamson <alex.williamson@hp.com>
Date: Wed Feb 4 09:02:45 2009 +0000
virtio_net: Add a MAC filter table
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
--- a/drivers/net/virtio_net.c 2013-12-09 16:12:03.897891975 -0800
+++ b/drivers/net/virtio_net.c 2013-12-09 16:12:36.353164803 -0800
@@ -893,7 +893,7 @@ static bool virtnet_send_command(struct
sg_init_one(&hdr, &ctrl, sizeof(ctrl));
sgs[out_num++] = &hdr;
- if (out)
+ for (; out; out = sg_next(out))
sgs[out_num++] = out;
if (in)
sgs[out_num + in_num++] = in;
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command
2013-12-10 0:16 [PATCH net-next 1/3] virtio_net: set multicast filter list to host Stephen Hemminger
@ 2013-12-10 0:17 ` Stephen Hemminger
2013-12-10 15:25 ` Michael S. Tsirkin
2013-12-11 3:28 ` David Miller
2013-12-10 0:18 ` [PATCH net-next 3/3] virtio_net: spelling fixes Stephen Hemminger
` (4 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: Stephen Hemminger @ 2013-12-10 0:17 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, virtualization, David Miller, Michael S. Tsirkin
All the code passes NULL for the last sg list (in).
Simplify by just removing it.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
--- a/drivers/net/virtio_net.c 2013-12-09 16:12:36.353164803 -0800
+++ b/drivers/net/virtio_net.c 2013-12-09 16:12:41.409051865 -0800
@@ -876,13 +876,12 @@ static netdev_tx_t start_xmit(struct sk_
* never fail unless improperly formated.
*/
static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
- struct scatterlist *out,
- struct scatterlist *in)
+ struct scatterlist *out)
{
struct scatterlist *sgs[4], hdr, stat;
struct virtio_net_ctrl_hdr ctrl;
virtio_net_ctrl_ack status = ~0;
- unsigned out_num = 0, in_num = 0, tmp;
+ unsigned out_num = 0, tmp;
/* Caller should know better */
BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
@@ -895,16 +894,13 @@ static bool virtnet_send_command(struct
for (; out; out = sg_next(out))
sgs[out_num++] = out;
- if (in)
- sgs[out_num + in_num++] = in;
/* Add return status. */
sg_init_one(&stat, &status, sizeof(status));
- sgs[out_num + in_num++] = &stat;
+ sgs[out_num] = &stat;
- BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
- BUG_ON(virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC)
- < 0);
+ BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
+ BUG_ON(virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC) < 0);
if (unlikely(!virtqueue_kick(vi->cvq)))
return status == VIRTIO_NET_OK;
@@ -934,8 +930,7 @@ static int virtnet_set_mac_address(struc
if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
sg_init_one(&sg, addr->sa_data, dev->addr_len);
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
- VIRTIO_NET_CTRL_MAC_ADDR_SET,
- &sg, NULL)) {
+ VIRTIO_NET_CTRL_MAC_ADDR_SET, &sg)) {
dev_warn(&vdev->dev,
"Failed to set mac address by vq command.\n");
return -EINVAL;
@@ -1008,7 +1003,7 @@ static void virtnet_ack_link_announce(st
{
rtnl_lock();
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_ANNOUNCE,
- VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL, NULL))
+ VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL))
dev_warn(&vi->dev->dev, "Failed to ack link announce.\n");
rtnl_unlock();
}
@@ -1026,7 +1021,7 @@ static int virtnet_set_queues(struct vir
sg_init_one(&sg, &s, sizeof(s));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
- VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg, NULL)) {
+ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg)) {
dev_warn(&dev->dev, "Fail to set num of queue pairs to %d\n",
queue_pairs);
return -EINVAL;
@@ -1076,16 +1071,14 @@ static void virtnet_set_rx_mode(struct n
sg_init_one(sg, &promisc, sizeof(promisc));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
- VIRTIO_NET_CTRL_RX_PROMISC,
- sg, NULL))
+ VIRTIO_NET_CTRL_RX_PROMISC, sg))
dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
promisc ? "en" : "dis");
sg_init_one(sg, &allmulti, sizeof(allmulti));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
- VIRTIO_NET_CTRL_RX_ALLMULTI,
- sg, NULL))
+ VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
allmulti ? "en" : "dis");
@@ -1121,8 +1114,7 @@ static void virtnet_set_rx_mode(struct n
sizeof(mac_data->entries) + (mc_count * ETH_ALEN));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
- VIRTIO_NET_CTRL_MAC_TABLE_SET,
- sg, NULL))
+ VIRTIO_NET_CTRL_MAC_TABLE_SET, sg))
dev_warn(&dev->dev, "Failed to set MAC filter table.\n");
kfree(buf);
@@ -1137,7 +1129,7 @@ static int virtnet_vlan_rx_add_vid(struc
sg_init_one(&sg, &vid, sizeof(vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
- VIRTIO_NET_CTRL_VLAN_ADD, &sg, NULL))
+ VIRTIO_NET_CTRL_VLAN_ADD, &sg))
dev_warn(&dev->dev, "Failed to add VLAN ID %d.\n", vid);
return 0;
}
@@ -1151,7 +1143,7 @@ static int virtnet_vlan_rx_kill_vid(stru
sg_init_one(&sg, &vid, sizeof(vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
- VIRTIO_NET_CTRL_VLAN_DEL, &sg, NULL))
+ VIRTIO_NET_CTRL_VLAN_DEL, &sg))
dev_warn(&dev->dev, "Failed to kill VLAN ID %d.\n", vid);
return 0;
}
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 3/3] virtio_net: spelling fixes
2013-12-10 0:16 [PATCH net-next 1/3] virtio_net: set multicast filter list to host Stephen Hemminger
2013-12-10 0:17 ` [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command Stephen Hemminger
@ 2013-12-10 0:18 ` Stephen Hemminger
2013-12-10 15:25 ` Michael S. Tsirkin
2013-12-11 3:28 ` David Miller
2013-12-10 15:24 ` [PATCH net-next 1/3] virtio_net: set multicast filter list to host Michael S. Tsirkin
` (3 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: Stephen Hemminger @ 2013-12-10 0:18 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, virtualization, David Miller, Michael S. Tsirkin
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
--- a/drivers/net/virtio_net.c 2013-12-09 16:12:41.409051865 -0800
+++ b/drivers/net/virtio_net.c 2013-12-09 16:12:43.872996856 -0800
@@ -873,7 +873,7 @@ static netdev_tx_t start_xmit(struct sk_
/*
* Send command via the control virtqueue and check status. Commands
* supported by the hypervisor, as indicated by feature bits, should
- * never fail unless improperly formated.
+ * never fail unless improperly formatted.
*/
static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
struct scatterlist *out)
@@ -1061,7 +1061,7 @@ static void virtnet_set_rx_mode(struct n
void *buf;
int i;
- /* We can't dynamicaly set ndo_set_rx_mode, so return gracefully */
+ /* We can't dynamically set ndo_set_rx_mode, so return gracefully */
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
return;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/3] virtio_net: set multicast filter list to host
2013-12-10 0:16 [PATCH net-next 1/3] virtio_net: set multicast filter list to host Stephen Hemminger
2013-12-10 0:17 ` [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command Stephen Hemminger
2013-12-10 0:18 ` [PATCH net-next 3/3] virtio_net: spelling fixes Stephen Hemminger
@ 2013-12-10 15:24 ` Michael S. Tsirkin
2013-12-10 21:07 ` Michael S. Tsirkin
2013-12-10 17:40 ` Vlad Yasevich
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2013-12-10 15:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Miller, virtualization
On Mon, Dec 09, 2013 at 04:16:32PM -0800, Stephen Hemminger wrote:
> The virtio_net driver never sends the multicast address list to
> the host. This is because send command takes a pointer to scatter list
> to send but only inserts that one entry into the outgoing scatter list.
>
> This bug has been there since:
> commit f565a7c259d71cc186753653d978c646d2354b36
> Author: Alex Williamson <alex.williamson@hp.com>
> Date: Wed Feb 4 09:02:45 2009 +0000
>
> virtio_net: Add a MAC filter table
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Question is, can we detect a broken driver in the host somehow?
If not we'll need a feature bit for hosts to actually do
MC filtering.
> --- a/drivers/net/virtio_net.c 2013-12-09 16:12:03.897891975 -0800
> +++ b/drivers/net/virtio_net.c 2013-12-09 16:12:36.353164803 -0800
> @@ -893,7 +893,7 @@ static bool virtnet_send_command(struct
> sg_init_one(&hdr, &ctrl, sizeof(ctrl));
> sgs[out_num++] = &hdr;
>
> - if (out)
> + for (; out; out = sg_next(out))
> sgs[out_num++] = out;
> if (in)
> sgs[out_num + in_num++] = in;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command
2013-12-10 0:17 ` [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command Stephen Hemminger
@ 2013-12-10 15:25 ` Michael S. Tsirkin
2013-12-11 3:28 ` David Miller
1 sibling, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2013-12-10 15:25 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Miller, virtualization
On Mon, Dec 09, 2013 at 04:17:40PM -0800, Stephen Hemminger wrote:
> All the code passes NULL for the last sg list (in).
> Simplify by just removing it.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
When we need it we can re-add it.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> --- a/drivers/net/virtio_net.c 2013-12-09 16:12:36.353164803 -0800
> +++ b/drivers/net/virtio_net.c 2013-12-09 16:12:41.409051865 -0800
> @@ -876,13 +876,12 @@ static netdev_tx_t start_xmit(struct sk_
> * never fail unless improperly formated.
> */
> static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> - struct scatterlist *out,
> - struct scatterlist *in)
> + struct scatterlist *out)
> {
> struct scatterlist *sgs[4], hdr, stat;
> struct virtio_net_ctrl_hdr ctrl;
> virtio_net_ctrl_ack status = ~0;
> - unsigned out_num = 0, in_num = 0, tmp;
> + unsigned out_num = 0, tmp;
>
> /* Caller should know better */
> BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
> @@ -895,16 +894,13 @@ static bool virtnet_send_command(struct
>
> for (; out; out = sg_next(out))
> sgs[out_num++] = out;
> - if (in)
> - sgs[out_num + in_num++] = in;
>
> /* Add return status. */
> sg_init_one(&stat, &status, sizeof(status));
> - sgs[out_num + in_num++] = &stat;
> + sgs[out_num] = &stat;
>
> - BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
> - BUG_ON(virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC)
> - < 0);
> + BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> + BUG_ON(virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC) < 0);
>
> if (unlikely(!virtqueue_kick(vi->cvq)))
> return status == VIRTIO_NET_OK;
> @@ -934,8 +930,7 @@ static int virtnet_set_mac_address(struc
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
> sg_init_one(&sg, addr->sa_data, dev->addr_len);
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
> - VIRTIO_NET_CTRL_MAC_ADDR_SET,
> - &sg, NULL)) {
> + VIRTIO_NET_CTRL_MAC_ADDR_SET, &sg)) {
> dev_warn(&vdev->dev,
> "Failed to set mac address by vq command.\n");
> return -EINVAL;
> @@ -1008,7 +1003,7 @@ static void virtnet_ack_link_announce(st
> {
> rtnl_lock();
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_ANNOUNCE,
> - VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL, NULL))
> + VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL))
> dev_warn(&vi->dev->dev, "Failed to ack link announce.\n");
> rtnl_unlock();
> }
> @@ -1026,7 +1021,7 @@ static int virtnet_set_queues(struct vir
> sg_init_one(&sg, &s, sizeof(s));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> - VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg, NULL)) {
> + VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg)) {
> dev_warn(&dev->dev, "Fail to set num of queue pairs to %d\n",
> queue_pairs);
> return -EINVAL;
> @@ -1076,16 +1071,14 @@ static void virtnet_set_rx_mode(struct n
> sg_init_one(sg, &promisc, sizeof(promisc));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> - VIRTIO_NET_CTRL_RX_PROMISC,
> - sg, NULL))
> + VIRTIO_NET_CTRL_RX_PROMISC, sg))
> dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
> promisc ? "en" : "dis");
>
> sg_init_one(sg, &allmulti, sizeof(allmulti));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> - VIRTIO_NET_CTRL_RX_ALLMULTI,
> - sg, NULL))
> + VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
> dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
> allmulti ? "en" : "dis");
>
> @@ -1121,8 +1114,7 @@ static void virtnet_set_rx_mode(struct n
> sizeof(mac_data->entries) + (mc_count * ETH_ALEN));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
> - VIRTIO_NET_CTRL_MAC_TABLE_SET,
> - sg, NULL))
> + VIRTIO_NET_CTRL_MAC_TABLE_SET, sg))
> dev_warn(&dev->dev, "Failed to set MAC filter table.\n");
>
> kfree(buf);
> @@ -1137,7 +1129,7 @@ static int virtnet_vlan_rx_add_vid(struc
> sg_init_one(&sg, &vid, sizeof(vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> - VIRTIO_NET_CTRL_VLAN_ADD, &sg, NULL))
> + VIRTIO_NET_CTRL_VLAN_ADD, &sg))
> dev_warn(&dev->dev, "Failed to add VLAN ID %d.\n", vid);
> return 0;
> }
> @@ -1151,7 +1143,7 @@ static int virtnet_vlan_rx_kill_vid(stru
> sg_init_one(&sg, &vid, sizeof(vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> - VIRTIO_NET_CTRL_VLAN_DEL, &sg, NULL))
> + VIRTIO_NET_CTRL_VLAN_DEL, &sg))
> dev_warn(&dev->dev, "Failed to kill VLAN ID %d.\n", vid);
> return 0;
> }
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 3/3] virtio_net: spelling fixes
2013-12-10 0:18 ` [PATCH net-next 3/3] virtio_net: spelling fixes Stephen Hemminger
@ 2013-12-10 15:25 ` Michael S. Tsirkin
2013-12-11 3:28 ` David Miller
1 sibling, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2013-12-10 15:25 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Miller, virtualization
On Mon, Dec 09, 2013 at 04:18:45PM -0800, Stephen Hemminger wrote:
>
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> --- a/drivers/net/virtio_net.c 2013-12-09 16:12:41.409051865 -0800
> +++ b/drivers/net/virtio_net.c 2013-12-09 16:12:43.872996856 -0800
> @@ -873,7 +873,7 @@ static netdev_tx_t start_xmit(struct sk_
> /*
> * Send command via the control virtqueue and check status. Commands
> * supported by the hypervisor, as indicated by feature bits, should
> - * never fail unless improperly formated.
> + * never fail unless improperly formatted.
> */
> static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> struct scatterlist *out)
> @@ -1061,7 +1061,7 @@ static void virtnet_set_rx_mode(struct n
> void *buf;
> int i;
>
> - /* We can't dynamicaly set ndo_set_rx_mode, so return gracefully */
> + /* We can't dynamically set ndo_set_rx_mode, so return gracefully */
> if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
> return;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/3] virtio_net: set multicast filter list to host
2013-12-10 0:16 [PATCH net-next 1/3] virtio_net: set multicast filter list to host Stephen Hemminger
` (2 preceding siblings ...)
2013-12-10 15:24 ` [PATCH net-next 1/3] virtio_net: set multicast filter list to host Michael S. Tsirkin
@ 2013-12-10 17:40 ` Vlad Yasevich
2013-12-11 3:28 ` David Miller
2013-12-12 18:26 ` David Miller
5 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-12-10 17:40 UTC (permalink / raw)
To: Stephen Hemminger, David Miller, Rusty Russell,
Michael S. Tsirkin
Cc: virtualization, netdev
On 12/09/2013 07:16 PM, Stephen Hemminger wrote:
> The virtio_net driver never sends the multicast address list to
> the host. This is because send command takes a pointer to scatter list
> to send but only inserts that one entry into the outgoing scatter list.
>
> This bug has been there since:
> commit f565a7c259d71cc186753653d978c646d2354b36
> Author: Alex Williamson <alex.williamson@hp.com>
> Date: Wed Feb 4 09:02:45 2009 +0000
>
> virtio_net: Add a MAC filter table
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
> --- a/drivers/net/virtio_net.c 2013-12-09 16:12:03.897891975 -0800
> +++ b/drivers/net/virtio_net.c 2013-12-09 16:12:36.353164803 -0800
> @@ -893,7 +893,7 @@ static bool virtnet_send_command(struct
> sg_init_one(&hdr, &ctrl, sizeof(ctrl));
> sgs[out_num++] = &hdr;
>
> - if (out)
> + for (; out; out = sg_next(out))
> sgs[out_num++] = out;
> if (in)
> sgs[out_num + in_num++] = in;
Hi Stephen
Amos Kong pointed out in your original thread, that this doesn't seem to
be needed and is working for him. I just checked my VMs and it's
working for me as well without this. Have you seen it fail?
Amos did a very good job explaining, but it basically boils down to the
fact that virtqueue_add_sgs() treats each entry of the sgs array
as a list and walks each list using exactly the same code as this patch
proposes :). virtqueue_add() also walks each array element as a list.
-vlad
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/3] virtio_net: set multicast filter list to host
2013-12-10 15:24 ` [PATCH net-next 1/3] virtio_net: set multicast filter list to host Michael S. Tsirkin
@ 2013-12-10 21:07 ` Michael S. Tsirkin
0 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2013-12-10 21:07 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Miller, virtualization
On Tue, Dec 10, 2013 at 05:24:29PM +0200, Michael S. Tsirkin wrote:
> On Mon, Dec 09, 2013 at 04:16:32PM -0800, Stephen Hemminger wrote:
> > The virtio_net driver never sends the multicast address list to
> > the host. This is because send command takes a pointer to scatter list
> > to send but only inserts that one entry into the outgoing scatter list.
> >
> > This bug has been there since:
> > commit f565a7c259d71cc186753653d978c646d2354b36
> > Author: Alex Williamson <alex.williamson@hp.com>
> > Date: Wed Feb 4 09:02:45 2009 +0000
> >
> > virtio_net: Add a MAC filter table
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Question is, can we detect a broken driver in the host somehow?
> If not we'll need a feature bit for hosts to actually do
> MC filtering.
Hmm Vlad and Amos made some good points about this patch.
Dave, pls don't apply yet until these are resolved.
> > --- a/drivers/net/virtio_net.c 2013-12-09 16:12:03.897891975 -0800
> > +++ b/drivers/net/virtio_net.c 2013-12-09 16:12:36.353164803 -0800
> > @@ -893,7 +893,7 @@ static bool virtnet_send_command(struct
> > sg_init_one(&hdr, &ctrl, sizeof(ctrl));
> > sgs[out_num++] = &hdr;
> >
> > - if (out)
> > + for (; out; out = sg_next(out))
> > sgs[out_num++] = out;
> > if (in)
> > sgs[out_num + in_num++] = in;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command
2013-12-10 0:17 ` [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command Stephen Hemminger
2013-12-10 15:25 ` Michael S. Tsirkin
@ 2013-12-11 3:28 ` David Miller
1 sibling, 0 replies; 12+ messages in thread
From: David Miller @ 2013-12-11 3:28 UTC (permalink / raw)
To: stephen; +Cc: rusty, mst, virtualization, netdev
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 9 Dec 2013 16:17:40 -0800
> All the code passes NULL for the last sg list (in).
> Simplify by just removing it.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 3/3] virtio_net: spelling fixes
2013-12-10 0:18 ` [PATCH net-next 3/3] virtio_net: spelling fixes Stephen Hemminger
2013-12-10 15:25 ` Michael S. Tsirkin
@ 2013-12-11 3:28 ` David Miller
1 sibling, 0 replies; 12+ messages in thread
From: David Miller @ 2013-12-11 3:28 UTC (permalink / raw)
To: stephen; +Cc: netdev, virtualization, mst
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 9 Dec 2013 16:18:45 -0800
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/3] virtio_net: set multicast filter list to host
2013-12-10 0:16 [PATCH net-next 1/3] virtio_net: set multicast filter list to host Stephen Hemminger
` (3 preceding siblings ...)
2013-12-10 17:40 ` Vlad Yasevich
@ 2013-12-11 3:28 ` David Miller
2013-12-12 18:26 ` David Miller
5 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-12-11 3:28 UTC (permalink / raw)
To: stephen; +Cc: netdev, virtualization, mst
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 9 Dec 2013 16:16:32 -0800
> The virtio_net driver never sends the multicast address list to
> the host. This is because send command takes a pointer to scatter list
> to send but only inserts that one entry into the outgoing scatter list.
>
> This bug has been there since:
> commit f565a7c259d71cc186753653d978c646d2354b36
> Author: Alex Williamson <alex.williamson@hp.com>
> Date: Wed Feb 4 09:02:45 2009 +0000
>
> virtio_net: Add a MAC filter table
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
This seems to still be under discussion so I'll hold off on this
one for now.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/3] virtio_net: set multicast filter list to host
2013-12-10 0:16 [PATCH net-next 1/3] virtio_net: set multicast filter list to host Stephen Hemminger
` (4 preceding siblings ...)
2013-12-11 3:28 ` David Miller
@ 2013-12-12 18:26 ` David Miller
5 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-12-12 18:26 UTC (permalink / raw)
To: stephen; +Cc: rusty, mst, virtualization, netdev
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 9 Dec 2013 16:16:32 -0800
> The virtio_net driver never sends the multicast address list to
> the host. This is because send command takes a pointer to scatter list
> to send but only inserts that one entry into the outgoing scatter list.
>
> This bug has been there since:
> commit f565a7c259d71cc186753653d978c646d2354b36
> Author: Alex Williamson <alex.williamson@hp.com>
> Date: Wed Feb 4 09:02:45 2009 +0000
>
> virtio_net: Add a MAC filter table
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
I'm just going to assume for now that you agree with the anaylsis of
others that the sg lists are processed properly as-is, and therefore
your patch isn't necessary.
Let me know if that is not the case.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-12-12 18:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-10 0:16 [PATCH net-next 1/3] virtio_net: set multicast filter list to host Stephen Hemminger
2013-12-10 0:17 ` [PATCH net-next 2/3] virtio_net: remove unused parameter to send_command Stephen Hemminger
2013-12-10 15:25 ` Michael S. Tsirkin
2013-12-11 3:28 ` David Miller
2013-12-10 0:18 ` [PATCH net-next 3/3] virtio_net: spelling fixes Stephen Hemminger
2013-12-10 15:25 ` Michael S. Tsirkin
2013-12-11 3:28 ` David Miller
2013-12-10 15:24 ` [PATCH net-next 1/3] virtio_net: set multicast filter list to host Michael S. Tsirkin
2013-12-10 21:07 ` Michael S. Tsirkin
2013-12-10 17:40 ` Vlad Yasevich
2013-12-11 3:28 ` David Miller
2013-12-12 18:26 ` David Miller
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 NNTP newsgroup(s).