* Re: [PATCH net-next] net: virtio: use eth_hw_addr_set() [not found] <20211026175634.3198477-1-kuba@kernel.org> @ 2021-10-27 2:45 ` Jason Wang 2021-10-27 7:24 ` Jason Wang 2021-10-27 7:23 ` Michael S. Tsirkin 1 sibling, 1 reply; 5+ messages in thread From: Jason Wang @ 2021-10-27 2:45 UTC (permalink / raw) To: Jakub Kicinski; +Cc: netdev, virtualization, davem, mst On Wed, Oct 27, 2021 at 1:56 AM Jakub Kicinski <kuba@kernel.org> wrote: > > Commit 406f42fa0d3c ("net-next: When a bond have a massive amount > of VLANs...") introduced a rbtree for faster Ethernet address look > up. To maintain netdev->dev_addr in this tree we need to make all > the writes to it go through appropriate helpers. I think the title should be "net: virtio: use eth_hw_addr_set()" > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: mst@redhat.com > CC: jasowang@redhat.com > CC: virtualization@lists.linux-foundation.org > --- > drivers/net/virtio_net.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index c501b5974aee..b7f35aff8e82 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -3177,12 +3177,16 @@ static int virtnet_probe(struct virtio_device *vdev) > dev->max_mtu = MAX_MTU; > > /* Configuration may specify what MAC to use. Otherwise random. */ > - if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) { > + u8 addr[MAX_ADDR_LEN]; > + > virtio_cread_bytes(vdev, > offsetof(struct virtio_net_config, mac), > - dev->dev_addr, dev->addr_len); > - else > + addr, dev->addr_len); > + dev_addr_set(dev, addr); > + } else { > eth_hw_addr_random(dev); > + } Do we need to change virtnet_set_mac_address() as well? Thanks > > /* Set up our device-specific information */ > vi = netdev_priv(dev); > -- > 2.31.1 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: virtio: use eth_hw_addr_set() 2021-10-27 2:45 ` [PATCH net-next] net: virtio: use eth_hw_addr_set() Jason Wang @ 2021-10-27 7:24 ` Jason Wang 2021-10-27 7:40 ` Michael S. Tsirkin 0 siblings, 1 reply; 5+ messages in thread From: Jason Wang @ 2021-10-27 7:24 UTC (permalink / raw) To: Jakub Kicinski; +Cc: netdev, virtualization, davem, mst On Wed, Oct 27, 2021 at 10:45 AM Jason Wang <jasowang@redhat.com> wrote: > > On Wed, Oct 27, 2021 at 1:56 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > > Commit 406f42fa0d3c ("net-next: When a bond have a massive amount > > of VLANs...") introduced a rbtree for faster Ethernet address look > > up. To maintain netdev->dev_addr in this tree we need to make all > > the writes to it go through appropriate helpers. > > I think the title should be "net: virtio: use eth_hw_addr_set()" I meant "dev_addr_set()" actually. Thanks > > > > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > > --- > > CC: mst@redhat.com > > CC: jasowang@redhat.com > > CC: virtualization@lists.linux-foundation.org > > --- > > drivers/net/virtio_net.c | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index c501b5974aee..b7f35aff8e82 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -3177,12 +3177,16 @@ static int virtnet_probe(struct virtio_device *vdev) > > dev->max_mtu = MAX_MTU; > > > > /* Configuration may specify what MAC to use. Otherwise random. */ > > - if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) > > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) { > > + u8 addr[MAX_ADDR_LEN]; > > + > > virtio_cread_bytes(vdev, > > offsetof(struct virtio_net_config, mac), > > - dev->dev_addr, dev->addr_len); > > - else > > + addr, dev->addr_len); > > + dev_addr_set(dev, addr); > > + } else { > > eth_hw_addr_random(dev); > > + } > > Do we need to change virtnet_set_mac_address() as well? > > Thanks > > > > > /* Set up our device-specific information */ > > vi = netdev_priv(dev); > > -- > > 2.31.1 > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: virtio: use eth_hw_addr_set() 2021-10-27 7:24 ` Jason Wang @ 2021-10-27 7:40 ` Michael S. Tsirkin 0 siblings, 0 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2021-10-27 7:40 UTC (permalink / raw) To: Jason Wang; +Cc: Jakub Kicinski, virtualization, davem, netdev On Wed, Oct 27, 2021 at 03:24:55PM +0800, Jason Wang wrote: > On Wed, Oct 27, 2021 at 10:45 AM Jason Wang <jasowang@redhat.com> wrote: > > > > On Wed, Oct 27, 2021 at 1:56 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > > > > Commit 406f42fa0d3c ("net-next: When a bond have a massive amount > > > of VLANs...") introduced a rbtree for faster Ethernet address look > > > up. To maintain netdev->dev_addr in this tree we need to make all > > > the writes to it go through appropriate helpers. > > > > I think the title should be "net: virtio: use eth_hw_addr_set()" > > I meant "dev_addr_set()" actually. > > Thanks Good point, this confused me too. Could be fixed up when applying? > > > > > > > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > > > --- > > > CC: mst@redhat.com > > > CC: jasowang@redhat.com > > > CC: virtualization@lists.linux-foundation.org > > > --- > > > drivers/net/virtio_net.c | 10 +++++++--- > > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > > index c501b5974aee..b7f35aff8e82 100644 > > > --- a/drivers/net/virtio_net.c > > > +++ b/drivers/net/virtio_net.c > > > @@ -3177,12 +3177,16 @@ static int virtnet_probe(struct virtio_device *vdev) > > > dev->max_mtu = MAX_MTU; > > > > > > /* Configuration may specify what MAC to use. Otherwise random. */ > > > - if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) > > > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) { > > > + u8 addr[MAX_ADDR_LEN]; > > > + > > > virtio_cread_bytes(vdev, > > > offsetof(struct virtio_net_config, mac), > > > - dev->dev_addr, dev->addr_len); > > > - else > > > + addr, dev->addr_len); > > > + dev_addr_set(dev, addr); > > > + } else { > > > eth_hw_addr_random(dev); > > > + } > > > > Do we need to change virtnet_set_mac_address() as well? > > > > Thanks > > > > > > > > /* Set up our device-specific information */ > > > vi = netdev_priv(dev); > > > -- > > > 2.31.1 > > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: virtio: use eth_hw_addr_set() [not found] <20211026175634.3198477-1-kuba@kernel.org> 2021-10-27 2:45 ` [PATCH net-next] net: virtio: use eth_hw_addr_set() Jason Wang @ 2021-10-27 7:23 ` Michael S. Tsirkin [not found] ` <20211027062640.5d32d7be@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> 1 sibling, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2021-10-27 7:23 UTC (permalink / raw) To: Jakub Kicinski; +Cc: netdev, davem, virtualization On Tue, Oct 26, 2021 at 10:56:34AM -0700, Jakub Kicinski wrote: > Commit 406f42fa0d3c ("net-next: When a bond have a massive amount > of VLANs...") introduced a rbtree for faster Ethernet address look > up. To maintain netdev->dev_addr in this tree we need to make all > the writes to it go through appropriate helpers. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > CC: mst@redhat.com > CC: jasowang@redhat.com > CC: virtualization@lists.linux-foundation.org > --- > drivers/net/virtio_net.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index c501b5974aee..b7f35aff8e82 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -3177,12 +3177,16 @@ static int virtnet_probe(struct virtio_device *vdev) > dev->max_mtu = MAX_MTU; > > /* Configuration may specify what MAC to use. Otherwise random. */ > - if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) { > + u8 addr[MAX_ADDR_LEN]; > + > virtio_cread_bytes(vdev, > offsetof(struct virtio_net_config, mac), > - dev->dev_addr, dev->addr_len); > - else > + addr, dev->addr_len); Maybe BUG_ON(dev->addr_len > sizeof addr); here just to make sure we don't overflow addr silently? > + dev_addr_set(dev, addr); > + } else { > eth_hw_addr_random(dev); > + } > > /* Set up our device-specific information */ > vi = netdev_priv(dev); > -- > 2.31.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20211027062640.5d32d7be@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>]
* Re: [PATCH net-next] net: virtio: use eth_hw_addr_set() [not found] ` <20211027062640.5d32d7be@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> @ 2021-10-27 14:55 ` Michael S. Tsirkin 0 siblings, 0 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2021-10-27 14:55 UTC (permalink / raw) To: Jakub Kicinski; +Cc: netdev, davem, virtualization On Wed, Oct 27, 2021 at 06:26:40AM -0700, Jakub Kicinski wrote: > On Wed, 27 Oct 2021 03:23:17 -0400 Michael S. Tsirkin wrote: > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > > index c501b5974aee..b7f35aff8e82 100644 > > > --- a/drivers/net/virtio_net.c > > > +++ b/drivers/net/virtio_net.c > > > @@ -3177,12 +3177,16 @@ static int virtnet_probe(struct virtio_device *vdev) > > > dev->max_mtu = MAX_MTU; > > > > > > /* Configuration may specify what MAC to use. Otherwise random. */ > > > - if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) > > > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) { > > > + u8 addr[MAX_ADDR_LEN]; > > > + > > > virtio_cread_bytes(vdev, > > > offsetof(struct virtio_net_config, mac), > > > - dev->dev_addr, dev->addr_len); > > > - else > > > + addr, dev->addr_len); > > > > Maybe BUG_ON(dev->addr_len > sizeof addr); > > > > here just to make sure we don't overflow addr silently? > > Since I need to post a v2 and we're talking... can I just use > eth_hw_addr_set() instead? AFAICT netdev is always allocated with > alloc_etherdev_mq() and ->addr_len never changed. Plus there is > a number of Ethernet address helpers used so ETH_ALEN is kind of > already assumed. Right. Sure, just document this in the commit log pls. > > > + dev_addr_set(dev, addr); > > > + } else { > > > eth_hw_addr_random(dev); > > > + } > > > > > > /* Set up our device-specific information */ > > > vi = netdev_priv(dev); _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-27 14:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20211026175634.3198477-1-kuba@kernel.org>
2021-10-27 2:45 ` [PATCH net-next] net: virtio: use eth_hw_addr_set() Jason Wang
2021-10-27 7:24 ` Jason Wang
2021-10-27 7:40 ` Michael S. Tsirkin
2021-10-27 7:23 ` Michael S. Tsirkin
[not found] ` <20211027062640.5d32d7be@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
2021-10-27 14:55 ` Michael S. Tsirkin
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).