* [RFC] virtio-net: check the mac address for vdpa device
@ 2024-07-09 2:27 Cindy Lu
2024-07-09 2:33 ` Jason Wang
0 siblings, 1 reply; 6+ messages in thread
From: Cindy Lu @ 2024-07-09 2:27 UTC (permalink / raw)
To: lulu, dtatulea, mst, jasowang, parav, qemu-devel
When using VDPA device, we should verify whether the MAC address in the
hardware matches the MAC address from the QEMU command line. If not,
we will need to update the related information.
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/net/virtio-net.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9c7e85caea..db04331b82 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
nc->rxfilter_notify_enabled = 1;
if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
- struct virtio_net_config netcfg = {};
- memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
- vhost_net_set_config(get_vhost_net(nc->peer),
- (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
+ struct virtio_net_config netcfg = {};
+ static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
+ vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
+ ETH_ALEN);
+ if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
+ memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
+ memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
+ memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
+ }
+ vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
+ ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
}
QTAILQ_INIT(&n->rsc_chains);
n->qdev = dev;
--
2.45.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] virtio-net: check the mac address for vdpa device
2024-07-09 2:27 [RFC] virtio-net: check the mac address for vdpa device Cindy Lu
@ 2024-07-09 2:33 ` Jason Wang
2024-07-09 2:40 ` Cindy Lu
0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2024-07-09 2:33 UTC (permalink / raw)
To: Cindy Lu; +Cc: dtatulea, mst, parav, qemu-devel
On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
>
> When using VDPA device, we should verify whether the MAC address in the
> hardware matches the MAC address from the QEMU command line. If not,
> we will need to update the related information.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
This seems to be a workaround, for example it means the mac address
set from the qemu command line has the higher priority compared to the
one that is provisioned by the host?
At least we need to have a warning here?
Thanks
> ---
> hw/net/virtio-net.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9c7e85caea..db04331b82 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> nc->rxfilter_notify_enabled = 1;
>
> if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> - struct virtio_net_config netcfg = {};
> - memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> - vhost_net_set_config(get_vhost_net(nc->peer),
> - (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> + struct virtio_net_config netcfg = {};
> + static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> + vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> + ETH_ALEN);
> + if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> + memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> + memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> + memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> + }
> + vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> + ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> }
> QTAILQ_INIT(&n->rsc_chains);
> n->qdev = dev;
> --
> 2.45.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] virtio-net: check the mac address for vdpa device
2024-07-09 2:33 ` Jason Wang
@ 2024-07-09 2:40 ` Cindy Lu
2024-07-09 2:47 ` Jason Wang
0 siblings, 1 reply; 6+ messages in thread
From: Cindy Lu @ 2024-07-09 2:40 UTC (permalink / raw)
To: Jason Wang; +Cc: dtatulea, mst, parav, qemu-devel
On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > When using VDPA device, we should verify whether the MAC address in the
> > hardware matches the MAC address from the QEMU command line. If not,
> > we will need to update the related information.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
>
> This seems to be a workaround, for example it means the mac address
> set from the qemu command line has the higher priority compared to the
> one that is provisioned by the host?
>
> At least we need to have a warning here?
>
In this design, the MAC address from the host will take higher
priority over the MAC address from the command line. I'm not sure if
this is acceptable?
sure, will add some warning here
> Thanks
>
> > ---
> > hw/net/virtio-net.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 9c7e85caea..db04331b82 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> > nc->rxfilter_notify_enabled = 1;
> >
> > if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > - struct virtio_net_config netcfg = {};
> > - memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > - vhost_net_set_config(get_vhost_net(nc->peer),
> > - (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > + struct virtio_net_config netcfg = {};
> > + static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > + vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > + ETH_ALEN);
> > + if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > + memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > + memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > + memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > + }
> > + vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > + ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > }
> > QTAILQ_INIT(&n->rsc_chains);
> > n->qdev = dev;
> > --
> > 2.45.0
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] virtio-net: check the mac address for vdpa device
2024-07-09 2:40 ` Cindy Lu
@ 2024-07-09 2:47 ` Jason Wang
2024-07-09 2:55 ` Cindy Lu
0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2024-07-09 2:47 UTC (permalink / raw)
To: Cindy Lu; +Cc: dtatulea, mst, parav, qemu-devel
On Tue, Jul 9, 2024 at 10:41 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > When using VDPA device, we should verify whether the MAC address in the
> > > hardware matches the MAC address from the QEMU command line. If not,
> > > we will need to update the related information.
> > >
> > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> >
> > This seems to be a workaround, for example it means the mac address
> > set from the qemu command line has the higher priority compared to the
> > one that is provisioned by the host?
> >
> > At least we need to have a warning here?
> >
> In this design, the MAC address from the host will take higher
> priority over the MAC address from the command line. I'm not sure if
> this is acceptable?
That's fine but it seems not what I read for this patch?
As you try to set config, or anything I missed here?
Thanks
> sure, will add some warning here
> > Thanks
> >
> > > ---
> > > hw/net/virtio-net.c | 15 +++++++++++----
> > > 1 file changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index 9c7e85caea..db04331b82 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> > > nc->rxfilter_notify_enabled = 1;
> > >
> > > if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > - struct virtio_net_config netcfg = {};
> > > - memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > > - vhost_net_set_config(get_vhost_net(nc->peer),
> > > - (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > + struct virtio_net_config netcfg = {};
> > > + static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > > + vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > > + ETH_ALEN);
> > > + if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > > + memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > > + memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > > + memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > > + }
> > > + vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > > + ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > }
> > > QTAILQ_INIT(&n->rsc_chains);
> > > n->qdev = dev;
> > > --
> > > 2.45.0
> > >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] virtio-net: check the mac address for vdpa device
2024-07-09 2:47 ` Jason Wang
@ 2024-07-09 2:55 ` Cindy Lu
2024-07-09 3:00 ` Jason Wang
0 siblings, 1 reply; 6+ messages in thread
From: Cindy Lu @ 2024-07-09 2:55 UTC (permalink / raw)
To: Jason Wang; +Cc: dtatulea, mst, parav, qemu-devel
On Tue, 9 Jul 2024 at 10:47, Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Jul 9, 2024 at 10:41 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> > > >
> > > > When using VDPA device, we should verify whether the MAC address in the
> > > > hardware matches the MAC address from the QEMU command line. If not,
> > > > we will need to update the related information.
> > > >
> > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > >
> > > This seems to be a workaround, for example it means the mac address
> > > set from the qemu command line has the higher priority compared to the
> > > one that is provisioned by the host?
> > >
> > > At least we need to have a warning here?
> > >
> > In this design, the MAC address from the host will take higher
> > priority over the MAC address from the command line. I'm not sure if
> > this is acceptable?
>
> That's fine but it seems not what I read for this patch?
>
> As you try to set config, or anything I missed here?
>
> Thanks
>
yes I use vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
ETH_ALEN); to get the mac from hardware and copy it
to n->nic_conf.macaddr and n->mac
memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
qemu will use this information for the following steps
Thanks
cindy
> > sure, will add some warning here
> > > Thanks
> > >
> > > > ---
> > > > hw/net/virtio-net.c | 15 +++++++++++----
> > > > 1 file changed, 11 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > > index 9c7e85caea..db04331b82 100644
> > > > --- a/hw/net/virtio-net.c
> > > > +++ b/hw/net/virtio-net.c
> > > > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> > > > nc->rxfilter_notify_enabled = 1;
> > > >
> > > > if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > > - struct virtio_net_config netcfg = {};
> > > > - memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > > > - vhost_net_set_config(get_vhost_net(nc->peer),
> > > > - (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > + struct virtio_net_config netcfg = {};
> > > > + static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > > > + vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > > > + ETH_ALEN);
> > > > + if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > > > + memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > > > + memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > > > + memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > > > + }
> > > > + vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > > > + ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > }
> > > > QTAILQ_INIT(&n->rsc_chains);
> > > > n->qdev = dev;
> > > > --
> > > > 2.45.0
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] virtio-net: check the mac address for vdpa device
2024-07-09 2:55 ` Cindy Lu
@ 2024-07-09 3:00 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2024-07-09 3:00 UTC (permalink / raw)
To: Cindy Lu; +Cc: dtatulea, mst, parav, qemu-devel
On Tue, Jul 9, 2024 at 10:56 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, 9 Jul 2024 at 10:47, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Jul 9, 2024 at 10:41 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> > > > >
> > > > > When using VDPA device, we should verify whether the MAC address in the
> > > > > hardware matches the MAC address from the QEMU command line. If not,
> > > > > we will need to update the related information.
> > > > >
> > > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > >
> > > > This seems to be a workaround, for example it means the mac address
> > > > set from the qemu command line has the higher priority compared to the
> > > > one that is provisioned by the host?
> > > >
> > > > At least we need to have a warning here?
> > > >
> > > In this design, the MAC address from the host will take higher
> > > priority over the MAC address from the command line. I'm not sure if
> > > this is acceptable?
> >
> > That's fine but it seems not what I read for this patch?
> >
> > As you try to set config, or anything I missed here?
> >
> > Thanks
> >
> yes I use vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> ETH_ALEN); to get the mac from hardware and copy it
> to n->nic_conf.macaddr and n->mac
> memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> qemu will use this information for the following steps
I think we need to explain:
1) why do we need a get_config in realization, or what happens if we
don't do that. (AFAIK we will call get_config when a guest is trying
to read the config space).
2) why do we need a set_config, what happens if we don't do that.
Thanks
>
> Thanks
> cindy
>
> > > sure, will add some warning here
> > > > Thanks
> > > >
> > > > > ---
> > > > > hw/net/virtio-net.c | 15 +++++++++++----
> > > > > 1 file changed, 11 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > > > index 9c7e85caea..db04331b82 100644
> > > > > --- a/hw/net/virtio-net.c
> > > > > +++ b/hw/net/virtio-net.c
> > > > > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> > > > > nc->rxfilter_notify_enabled = 1;
> > > > >
> > > > > if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > > > - struct virtio_net_config netcfg = {};
> > > > > - memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > > > > - vhost_net_set_config(get_vhost_net(nc->peer),
> > > > > - (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > > + struct virtio_net_config netcfg = {};
> > > > > + static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > > > > + vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > > > > + ETH_ALEN);
> > > > > + if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > > > > + memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > > > > + memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > > > > + memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > > > > + }
> > > > > + vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > > > > + ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > > }
> > > > > QTAILQ_INIT(&n->rsc_chains);
> > > > > n->qdev = dev;
> > > > > --
> > > > > 2.45.0
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-09 3:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-09 2:27 [RFC] virtio-net: check the mac address for vdpa device Cindy Lu
2024-07-09 2:33 ` Jason Wang
2024-07-09 2:40 ` Cindy Lu
2024-07-09 2:47 ` Jason Wang
2024-07-09 2:55 ` Cindy Lu
2024-07-09 3:00 ` 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).