* [PATCH v2] virtio-net: Add check for mac address while peer is vdpa
@ 2020-10-23 9:15 Cindy Lu
2020-10-26 2:43 ` Jason Wang
0 siblings, 1 reply; 5+ messages in thread
From: Cindy Lu @ 2020-10-23 9:15 UTC (permalink / raw)
To: mst, jasowang, qemu-devel; +Cc: qemu-stable, Cindy Lu
Sometime vdpa get an all 0 mac address from the hardware, this will cause the traffic down
So we add the check for this part.
if we get an 0 mac address we will use the default mac address instead
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/net/virtio-net.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9179013ac4..f1648fc47d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
VirtIONet *n = VIRTIO_NET(vdev);
struct virtio_net_config netcfg;
NetClientState *nc = qemu_get_queue(n->nic);
+ static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
int ret = 0;
memset(&netcfg, 0 , sizeof(struct virtio_net_config));
@@ -151,7 +152,11 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
n->config_size);
if (ret != -1) {
- memcpy(config, &netcfg, n->config_size);
+ if (memcmp(&netcfg.mac, &zero, sizeof(zero)) != 0) {
+ memcpy(config, &netcfg, n->config_size);
+ } else {
+ error_report("Get an all zero mac address from hardware");
+ }
}
}
}
--
2.21.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] virtio-net: Add check for mac address while peer is vdpa
2020-10-23 9:15 [PATCH v2] virtio-net: Add check for mac address while peer is vdpa Cindy Lu
@ 2020-10-26 2:43 ` Jason Wang
2020-10-26 3:19 ` Jason Wang
0 siblings, 1 reply; 5+ messages in thread
From: Jason Wang @ 2020-10-26 2:43 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel; +Cc: qemu-stable
On 2020/10/23 下午5:15, Cindy Lu wrote:
> Sometime vdpa get an all 0 mac address from the hardware, this will cause the traffic down
> So we add the check for this part.
> if we get an 0 mac address we will use the default mac address instead
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/net/virtio-net.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9179013ac4..f1648fc47d 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> VirtIONet *n = VIRTIO_NET(vdev);
> struct virtio_net_config netcfg;
> NetClientState *nc = qemu_get_queue(n->nic);
> + static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
>
> int ret = 0;
> memset(&netcfg, 0 , sizeof(struct virtio_net_config));
> @@ -151,7 +152,11 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> n->config_size);
> if (ret != -1) {
> - memcpy(config, &netcfg, n->config_size);
> + if (memcmp(&netcfg.mac, &zero, sizeof(zero)) != 0) {
> + memcpy(config, &netcfg, n->config_size);
> + } else {
> + error_report("Get an all zero mac address from hardware");
> + }
> }
> }
> }
Applied.
Thanks
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] virtio-net: Add check for mac address while peer is vdpa
2020-10-26 2:43 ` Jason Wang
@ 2020-10-26 3:19 ` Jason Wang
2020-10-26 4:41 ` Cindy Lu
0 siblings, 1 reply; 5+ messages in thread
From: Jason Wang @ 2020-10-26 3:19 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel; +Cc: qemu-stable
On 2020/10/26 上午10:43, Jason Wang wrote:
>
> On 2020/10/23 下午5:15, Cindy Lu wrote:
>> Sometime vdpa get an all 0 mac address from the hardware, this will
>> cause the traffic down
>> So we add the check for this part.
>> if we get an 0 mac address we will use the default mac address instead
>>
>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>> ---
>> hw/net/virtio-net.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 9179013ac4..f1648fc47d 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice
>> *vdev, uint8_t *config)
>> VirtIONet *n = VIRTIO_NET(vdev);
>> struct virtio_net_config netcfg;
>> NetClientState *nc = qemu_get_queue(n->nic);
>> + static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
>> int ret = 0;
>> memset(&netcfg, 0 , sizeof(struct virtio_net_config));
>> @@ -151,7 +152,11 @@ static void virtio_net_get_config(VirtIODevice
>> *vdev, uint8_t *config)
>> ret = vhost_net_get_config(get_vhost_net(nc->peer),
>> (uint8_t *)&netcfg,
>> n->config_size);
>> if (ret != -1) {
>> - memcpy(config, &netcfg, n->config_size);
>> + if (memcmp(&netcfg.mac, &zero, sizeof(zero)) != 0) {
>> + memcpy(config, &netcfg, n->config_size);
>> + } else {
>> + error_report("Get an all zero mac address from
>> hardware");
>> + }
>> }
>> }
>> }
>
>
> Applied.
>
> Thanks
>
>
Speak too fast. Some questions:
1) Any reason that you do such check or get_config() instead of doing it
once in device initalization
2) the indentation looks wrong
3) There's no need for an error here since we can workaround it
Thanks
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] virtio-net: Add check for mac address while peer is vdpa
2020-10-26 3:19 ` Jason Wang
@ 2020-10-26 4:41 ` Cindy Lu
2020-10-27 2:42 ` Jason Wang
0 siblings, 1 reply; 5+ messages in thread
From: Cindy Lu @ 2020-10-26 4:41 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-stable, QEMU Developers, Michael Tsirkin
[-- Attachment #1: Type: text/plain, Size: 2629 bytes --]
Hi jason
On Mon, Oct 26, 2020 at 11:20 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On 2020/10/26 上午10:43, Jason Wang wrote:
> >
> > On 2020/10/23 下午5:15, Cindy Lu wrote:
> >> Sometime vdpa get an all 0 mac address from the hardware, this will
> >> cause the traffic down
> >> So we add the check for this part.
> >> if we get an 0 mac address we will use the default mac address instead
> >>
> >> Signed-off-by: Cindy Lu <lulu@redhat.com>
> >> ---
> >> hw/net/virtio-net.c | 7 ++++++-
> >> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index 9179013ac4..f1648fc47d 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice
> >> *vdev, uint8_t *config)
> >> VirtIONet *n = VIRTIO_NET(vdev);
> >> struct virtio_net_config netcfg;
> >> NetClientState *nc = qemu_get_queue(n->nic);
> >> + static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
> >> int ret = 0;
> >> memset(&netcfg, 0 , sizeof(struct virtio_net_config));
> >> @@ -151,7 +152,11 @@ static void virtio_net_get_config(VirtIODevice
> >> *vdev, uint8_t *config)
> >> ret = vhost_net_get_config(get_vhost_net(nc->peer),
> >> (uint8_t *)&netcfg,
> >> n->config_size);
> >> if (ret != -1) {
> >> - memcpy(config, &netcfg, n->config_size);
> >> + if (memcmp(&netcfg.mac, &zero, sizeof(zero)) != 0) {
> >> + memcpy(config, &netcfg, n->config_size);
> >> + } else {
> >> + error_report("Get an all zero mac address from
> >> hardware");
> >> + }
> >> }
> >> }
> >> }
> >
> >
> > Applied.
> >
> > Thanks
> >
> >
>
> Speak too fast. Some questions:
>
> 1) Any reason that you do such check or get_config() instead of doing it
> once in device initalization
>
get_config() was called before the device realized. If we check in the
device initalization, the mac address is already overwritten to 0 and
we lost the default mac address
Also for my understanding. the mac address read from hardware should never
been 0, so we need to check it every time we got it, Just in case it will
change to 0 and overwrite the mac address while the qemu running or some
other case
> 2) the indentation looks wrong
>
sure will fix this
> 3) There's no need for an error here since we can workaround it
sure, I will change it to log
>
>
> Thanks
>
>
[-- Attachment #2: Type: text/html, Size: 4172 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] virtio-net: Add check for mac address while peer is vdpa
2020-10-26 4:41 ` Cindy Lu
@ 2020-10-27 2:42 ` Jason Wang
0 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2020-10-27 2:42 UTC (permalink / raw)
To: Cindy Lu; +Cc: Michael Tsirkin, qemu-stable, QEMU Developers
----- Original Message -----
> Hi jason
>
>
> On Mon, Oct 26, 2020 at 11:20 AM Jason Wang <jasowang@redhat.com> wrote:
>
> >
> > On 2020/10/26 上午10:43, Jason Wang wrote:
> > >
> > > On 2020/10/23 下午5:15, Cindy Lu wrote:
> > >> Sometime vdpa get an all 0 mac address from the hardware, this will
> > >> cause the traffic down
> > >> So we add the check for this part.
> > >> if we get an 0 mac address we will use the default mac address instead
> > >>
> > >> Signed-off-by: Cindy Lu <lulu@redhat.com>
> > >> ---
> > >> hw/net/virtio-net.c | 7 ++++++-
> > >> 1 file changed, 6 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > >> index 9179013ac4..f1648fc47d 100644
> > >> --- a/hw/net/virtio-net.c
> > >> +++ b/hw/net/virtio-net.c
> > >> @@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice
> > >> *vdev, uint8_t *config)
> > >> VirtIONet *n = VIRTIO_NET(vdev);
> > >> struct virtio_net_config netcfg;
> > >> NetClientState *nc = qemu_get_queue(n->nic);
> > >> + static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
> > >> int ret = 0;
> > >> memset(&netcfg, 0 , sizeof(struct virtio_net_config));
> > >> @@ -151,7 +152,11 @@ static void virtio_net_get_config(VirtIODevice
> > >> *vdev, uint8_t *config)
> > >> ret = vhost_net_get_config(get_vhost_net(nc->peer),
> > >> (uint8_t *)&netcfg,
> > >> n->config_size);
> > >> if (ret != -1) {
> > >> - memcpy(config, &netcfg, n->config_size);
> > >> + if (memcmp(&netcfg.mac, &zero, sizeof(zero)) != 0) {
> > >> + memcpy(config, &netcfg, n->config_size);
> > >> + } else {
> > >> + error_report("Get an all zero mac address from
> > >> hardware");
> > >> + }
> > >> }
> > >> }
> > >> }
> > >
> > >
> > > Applied.
> > >
> > > Thanks
> > >
> > >
> >
> > Speak too fast. Some questions:
> >
> > 1) Any reason that you do such check or get_config() instead of doing it
> > once in device initalization
> >
> get_config() was called before the device realized.
Intersting, any reason that we need call get_config() even before
device is realized?
> If we check in the
> device initalization, the mac address is already overwritten to 0 and
> we lost the default mac address
> Also for my understanding. the mac address read from hardware should never
> been 0, so we need to check it every time we got it,
This sounds self-contradict.
> Just in case it will
> change to 0 and overwrite the mac address while the qemu running or some
> other case
This looks like a bug in the device or driver.
Even if this is true, you can only detect it via get_config(). And I
don't think we need to workaround a buggy device.
Thanks
>
> > 2) the indentation looks wrong
> >
> sure will fix this
>
> > 3) There's no need for an error here since we can workaround it
>
> sure, I will change it to log
>
> >
> >
>
>
> > Thanks
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-27 2:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-23 9:15 [PATCH v2] virtio-net: Add check for mac address while peer is vdpa Cindy Lu
2020-10-26 2:43 ` Jason Wang
2020-10-26 3:19 ` Jason Wang
2020-10-26 4:41 ` Cindy Lu
2020-10-27 2:42 ` Jason Wang
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).