* [PATCH net-next v2 3/8] net/ncsi: Use struct sockaddr_storage for pending_mac
From: Kees Cook @ 2025-05-21 20:46 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Kees Cook, Gustavo A . R . Silva, Samuel Mendoza-Jonas,
Paul Fertser, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev, Willem de Bruijn,
Martin K. Petersen, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Andrew Lunn, Stanislav Fomichev,
Cosmin Ratiu, Lei Yang, Ido Schimmel, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Jason Wang, Vladimir Oltean,
Florian Fainelli, Kory Maincent, Maxim Georgiev,
Aleksander Jan Bajkowski, Philipp Hahn, Eric Biggers,
Ard Biesheuvel, Al Viro, Ahmed Zaki, Alexander Lobakin,
Xiao Liang, linux-kernel, linux-nvme, linux-scsi, target-devel,
linux-wpan, linux-usb, linux-hyperv, linux-hardening
In-Reply-To: <20250521204310.it.500-kees@kernel.org>
To avoid future casting with coming API type changes, switch struct
ncsi_dev_priv::pending_mac to a full struct sockaddr_storage.
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Cc: Paul Fertser <fercerpav@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: <netdev@vger.kernel.org>
---
net/ncsi/internal.h | 2 +-
net/ncsi/ncsi-manage.c | 2 +-
net/ncsi/ncsi-rsp.c | 18 +++++++++---------
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 2c260f33b55c..e76c6de0c784 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -322,7 +322,7 @@ struct ncsi_dev_priv {
#define NCSI_DEV_RESHUFFLE 4
#define NCSI_DEV_RESET 8 /* Reset state of NC */
unsigned int gma_flag; /* OEM GMA flag */
- struct sockaddr pending_mac; /* MAC address received from GMA */
+ struct sockaddr_storage pending_mac; /* MAC address received from GMA */
spinlock_t lock; /* Protect the NCSI device */
unsigned int package_probe_id;/* Current ID during probe */
unsigned int package_num; /* Number of packages */
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index b36947063783..0202db2aea3e 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1058,7 +1058,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_config_apply_mac:
rtnl_lock();
- ret = dev_set_mac_address(dev, &ndp->pending_mac, NULL);
+ ret = dev_set_mac_address(dev, (struct sockaddr *)&ndp->pending_mac, NULL);
rtnl_unlock();
if (ret < 0)
netdev_warn(dev, "NCSI: 'Writing MAC address to device failed\n");
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 8668888c5a2f..472cc68ad86f 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -628,7 +628,7 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
{
struct ncsi_dev_priv *ndp = nr->ndp;
- struct sockaddr *saddr = &ndp->pending_mac;
+ struct sockaddr_storage *saddr = &ndp->pending_mac;
struct net_device *ndev = ndp->ndev.dev;
struct ncsi_rsp_oem_pkt *rsp;
u32 mac_addr_off = 0;
@@ -644,11 +644,11 @@ static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
else if (mfr_id == NCSI_OEM_MFR_INTEL_ID)
mac_addr_off = INTEL_MAC_ADDR_OFFSET;
- saddr->sa_family = ndev->type;
- memcpy(saddr->sa_data, &rsp->data[mac_addr_off], ETH_ALEN);
+ saddr->ss_family = ndev->type;
+ memcpy(saddr->__data, &rsp->data[mac_addr_off], ETH_ALEN);
if (mfr_id == NCSI_OEM_MFR_BCM_ID || mfr_id == NCSI_OEM_MFR_INTEL_ID)
- eth_addr_inc((u8 *)saddr->sa_data);
- if (!is_valid_ether_addr((const u8 *)saddr->sa_data))
+ eth_addr_inc(saddr->__data);
+ if (!is_valid_ether_addr(saddr->__data))
return -ENXIO;
/* Set the flag for GMA command which should only be called once */
@@ -1088,7 +1088,7 @@ static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
{
struct ncsi_dev_priv *ndp = nr->ndp;
- struct sockaddr *saddr = &ndp->pending_mac;
+ struct sockaddr_storage *saddr = &ndp->pending_mac;
struct net_device *ndev = ndp->ndev.dev;
struct ncsi_rsp_gmcma_pkt *rsp;
int i;
@@ -1105,15 +1105,15 @@ static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
rsp->addresses[i][4], rsp->addresses[i][5]);
}
- saddr->sa_family = ndev->type;
+ saddr->ss_family = ndev->type;
for (i = 0; i < rsp->address_count; i++) {
if (!is_valid_ether_addr(rsp->addresses[i])) {
netdev_warn(ndev, "NCSI: Unable to assign %pM to device\n",
rsp->addresses[i]);
continue;
}
- memcpy(saddr->sa_data, rsp->addresses[i], ETH_ALEN);
- netdev_warn(ndev, "NCSI: Will set MAC address to %pM\n", saddr->sa_data);
+ memcpy(saddr->__data, rsp->addresses[i], ETH_ALEN);
+ netdev_warn(ndev, "NCSI: Will set MAC address to %pM\n", saddr->__data);
break;
}
--
2.34.1
^ permalink raw reply related
* [PATCH net-next v2 4/8] ieee802154: Use struct sockaddr_storage with dev_set_mac_address()
From: Kees Cook @ 2025-05-21 20:46 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Kees Cook, Gustavo A . R . Silva, Alexander Aring, Stefan Schmidt,
Miquel Raynal, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, linux-wpan, netdev, Willem de Bruijn,
Martin K. Petersen, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Andrew Lunn, Stanislav Fomichev,
Cosmin Ratiu, Lei Yang, Ido Schimmel, Samuel Mendoza-Jonas,
Paul Fertser, Hayes Wang, Douglas Anderson, Grant Grundler,
Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Jiri Pirko, Jason Wang, Vladimir Oltean,
Florian Fainelli, Kory Maincent, Maxim Georgiev,
Aleksander Jan Bajkowski, Philipp Hahn, Eric Biggers,
Ard Biesheuvel, Al Viro, Ahmed Zaki, Alexander Lobakin,
Xiao Liang, linux-kernel, linux-nvme, linux-scsi, target-devel,
linux-usb, linux-hyperv, linux-hardening
In-Reply-To: <20250521204310.it.500-kees@kernel.org>
Switch to struct sockaddr_storage for calling dev_set_mac_address(). Add
a temporary cast to struct sockaddr, which will be removed in a
subsequent patch.
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: <linux-wpan@vger.kernel.org>
Cc: <netdev@vger.kernel.org>
---
net/ieee802154/nl-phy.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
index 359249ab77bf..ee2b190e8e0d 100644
--- a/net/ieee802154/nl-phy.c
+++ b/net/ieee802154/nl-phy.c
@@ -224,17 +224,17 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
dev_hold(dev);
if (info->attrs[IEEE802154_ATTR_HW_ADDR]) {
- struct sockaddr addr;
+ struct sockaddr_storage addr;
- addr.sa_family = ARPHRD_IEEE802154;
- nla_memcpy(&addr.sa_data, info->attrs[IEEE802154_ATTR_HW_ADDR],
+ addr.ss_family = ARPHRD_IEEE802154;
+ nla_memcpy(&addr.__data, info->attrs[IEEE802154_ATTR_HW_ADDR],
IEEE802154_ADDR_LEN);
/* strangely enough, some callbacks (inetdev_event) from
* dev_set_mac_address require RTNL_LOCK
*/
rtnl_lock();
- rc = dev_set_mac_address(dev, &addr, NULL);
+ rc = dev_set_mac_address(dev, (struct sockaddr *)&addr, NULL);
rtnl_unlock();
if (rc)
goto dev_unregister;
--
2.34.1
^ permalink raw reply related
* [PATCH net-next v2 6/8] net: core: Convert dev_set_mac_address() to struct sockaddr_storage
From: Kees Cook @ 2025-05-21 20:46 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Kees Cook, Gustavo A . R . Silva, Jakub Kicinski, Jay Vosburgh,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Jiri Pirko,
Simon Horman, Alexander Aring, Stefan Schmidt, Miquel Raynal,
Samuel Mendoza-Jonas, Paul Fertser, Hayes Wang, Douglas Anderson,
Grant Grundler, Stanislav Fomichev, Cosmin Ratiu, Lei Yang,
netdev, linux-hyperv, linux-usb, linux-wpan, Willem de Bruijn,
Martin K. Petersen, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Ido Schimmel, Jason Wang, Vladimir Oltean,
Florian Fainelli, Kory Maincent, Maxim Georgiev,
Aleksander Jan Bajkowski, Philipp Hahn, Eric Biggers,
Ard Biesheuvel, Al Viro, Ahmed Zaki, Alexander Lobakin,
Xiao Liang, linux-kernel, linux-nvme, linux-scsi, target-devel,
linux-hardening
In-Reply-To: <20250521204310.it.500-kees@kernel.org>
All users of dev_set_mac_address() are now using a struct sockaddr_storage.
Convert the internal data type to struct sockaddr_storage, drop the casts,
and update pointer types.
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Simon Horman <horms@kernel.org>
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Cc: Paul Fertser <fercerpav@gmail.com>
Cc: Hayes Wang <hayeswang@realtek.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Grant Grundler <grundler@chromium.org>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Cosmin Ratiu <cratiu@nvidia.com>
Cc: Lei Yang <leiyang@redhat.com>
Cc: <netdev@vger.kernel.org>
Cc: <linux-hyperv@vger.kernel.org>
Cc: <linux-usb@vger.kernel.org>
Cc: <linux-wpan@vger.kernel.org>
---
include/linux/netdevice.h | 2 +-
drivers/net/bonding/bond_alb.c | 8 +++-----
drivers/net/bonding/bond_main.c | 15 ++++++---------
drivers/net/hyperv/netvsc_drv.c | 6 +++---
drivers/net/macvlan.c | 18 +++++++++---------
drivers/net/team/team_core.c | 2 +-
drivers/net/usb/r8152.c | 2 +-
net/core/dev.c | 1 +
net/core/dev_api.c | 6 +++---
net/ieee802154/nl-phy.c | 2 +-
net/ncsi/ncsi-manage.c | 2 +-
11 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 47200a394a02..b4242b997373 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4214,7 +4214,7 @@ int dev_pre_changeaddr_notify(struct net_device *dev, const char *addr,
struct netlink_ext_ack *extack);
int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
struct netlink_ext_ack *extack);
-int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
+int dev_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
struct netlink_ext_ack *extack);
int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
struct netlink_ext_ack *extack);
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 7edf0fd58c34..2d37b07c8215 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1035,7 +1035,7 @@ static int alb_set_slave_mac_addr(struct slave *slave, const u8 addr[],
*/
memcpy(ss.__data, addr, len);
ss.ss_family = dev->type;
- if (dev_set_mac_address(dev, (struct sockaddr *)&ss, NULL)) {
+ if (dev_set_mac_address(dev, &ss, NULL)) {
slave_err(slave->bond->dev, dev, "dev_set_mac_address on slave failed! ALB mode requires that the base driver support setting the hw address also when the network device's interface is open\n");
return -EOPNOTSUPP;
}
@@ -1273,8 +1273,7 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
break;
bond_hw_addr_copy(tmp_addr, rollback_slave->dev->dev_addr,
rollback_slave->dev->addr_len);
- dev_set_mac_address(rollback_slave->dev,
- (struct sockaddr *)&ss, NULL);
+ dev_set_mac_address(rollback_slave->dev, &ss, NULL);
dev_addr_set(rollback_slave->dev, tmp_addr);
}
@@ -1763,8 +1762,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
bond->dev->addr_len);
ss.ss_family = bond->dev->type;
/* we don't care if it can't change its mac, best effort */
- dev_set_mac_address(new_slave->dev, (struct sockaddr *)&ss,
- NULL);
+ dev_set_mac_address(new_slave->dev, &ss, NULL);
dev_addr_set(new_slave->dev, tmp_addr);
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 98cf4486fcee..c4d53e8e7c15 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1112,8 +1112,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
ss.ss_family = bond->dev->type;
}
- rv = dev_set_mac_address(new_active->dev,
- (struct sockaddr *)&ss, NULL);
+ rv = dev_set_mac_address(new_active->dev, &ss, NULL);
if (rv) {
slave_err(bond->dev, new_active->dev, "Error %d setting MAC of new active slave\n",
-rv);
@@ -1127,8 +1126,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
new_active->dev->addr_len);
ss.ss_family = old_active->dev->type;
- rv = dev_set_mac_address(old_active->dev,
- (struct sockaddr *)&ss, NULL);
+ rv = dev_set_mac_address(old_active->dev, &ss, NULL);
if (rv)
slave_err(bond->dev, old_active->dev, "Error %d setting MAC of old active slave\n",
-rv);
@@ -2127,7 +2125,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
}
ss.ss_family = slave_dev->type;
- res = dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, extack);
+ res = dev_set_mac_address(slave_dev, &ss, extack);
if (res) {
slave_err(bond_dev, slave_dev, "Error %d calling set_mac_address\n", res);
goto err_restore_mtu;
@@ -2455,7 +2453,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
bond_hw_addr_copy(ss.__data, new_slave->perm_hwaddr,
new_slave->dev->addr_len);
ss.ss_family = slave_dev->type;
- dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, NULL);
+ dev_set_mac_address(slave_dev, &ss, NULL);
}
err_restore_mtu:
@@ -2649,7 +2647,7 @@ static int __bond_release_one(struct net_device *bond_dev,
bond_hw_addr_copy(ss.__data, slave->perm_hwaddr,
slave->dev->addr_len);
ss.ss_family = slave_dev->type;
- dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, NULL);
+ dev_set_mac_address(slave_dev, &ss, NULL);
}
if (unregister) {
@@ -4936,8 +4934,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
if (rollback_slave == slave)
break;
- tmp_res = dev_set_mac_address(rollback_slave->dev,
- (struct sockaddr *)&tmp_ss, NULL);
+ tmp_res = dev_set_mac_address(rollback_slave->dev, &tmp_ss, NULL);
if (tmp_res) {
slave_dbg(bond_dev, rollback_slave->dev, "%s: unwind err %d\n",
__func__, tmp_res);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index d8b169ac0343..14a0d04e21ae 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1371,7 +1371,7 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
struct net_device_context *ndc = netdev_priv(ndev);
struct net_device *vf_netdev = rtnl_dereference(ndc->vf_netdev);
struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
- struct sockaddr *addr = p;
+ struct sockaddr_storage *addr = p;
int err;
err = eth_prepare_mac_addr_change(ndev, p);
@@ -1387,12 +1387,12 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
return err;
}
- err = rndis_filter_set_device_mac(nvdev, addr->sa_data);
+ err = rndis_filter_set_device_mac(nvdev, addr->__data);
if (!err) {
eth_commit_mac_addr_change(ndev, p);
} else if (vf_netdev) {
/* rollback change on VF */
- memcpy(addr->sa_data, ndev->dev_addr, ETH_ALEN);
+ memcpy(addr->__data, ndev->dev_addr, ETH_ALEN);
dev_set_mac_address(vf_netdev, addr, NULL);
}
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 7045b1d58754..4df991e494bd 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -754,13 +754,13 @@ static int macvlan_sync_address(struct net_device *dev,
static int macvlan_set_mac_address(struct net_device *dev, void *p)
{
struct macvlan_dev *vlan = netdev_priv(dev);
- struct sockaddr *addr = p;
+ struct sockaddr_storage *addr = p;
- if (!is_valid_ether_addr(addr->sa_data))
+ if (!is_valid_ether_addr(addr->__data))
return -EADDRNOTAVAIL;
/* If the addresses are the same, this is a no-op */
- if (ether_addr_equal(dev->dev_addr, addr->sa_data))
+ if (ether_addr_equal(dev->dev_addr, addr->__data))
return 0;
if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
@@ -768,10 +768,10 @@ static int macvlan_set_mac_address(struct net_device *dev, void *p)
return dev_set_mac_address(vlan->lowerdev, addr, NULL);
}
- if (macvlan_addr_busy(vlan->port, addr->sa_data))
+ if (macvlan_addr_busy(vlan->port, addr->__data))
return -EADDRINUSE;
- return macvlan_sync_address(dev, addr->sa_data);
+ return macvlan_sync_address(dev, addr->__data);
}
static void macvlan_change_rx_flags(struct net_device *dev, int change)
@@ -1295,11 +1295,11 @@ static void macvlan_port_destroy(struct net_device *dev)
*/
if (macvlan_passthru(port) &&
!ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
- struct sockaddr sa;
+ struct sockaddr_storage ss;
- sa.sa_family = port->dev->type;
- memcpy(&sa.sa_data, port->perm_addr, port->dev->addr_len);
- dev_set_mac_address(port->dev, &sa, NULL);
+ ss.ss_family = port->dev->type;
+ memcpy(&ss.__data, port->perm_addr, port->dev->addr_len);
+ dev_set_mac_address(port->dev, &ss, NULL);
}
kfree(port);
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index d8fc0c79745d..a64e661c21a1 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -55,7 +55,7 @@ static int __set_port_dev_addr(struct net_device *port_dev,
memcpy(addr.__data, dev_addr, port_dev->addr_len);
addr.ss_family = port_dev->type;
- return dev_set_mac_address(port_dev, (struct sockaddr *)&addr, NULL);
+ return dev_set_mac_address(port_dev, &addr, NULL);
}
static int team_port_set_orig_dev_addr(struct team_port *port)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b18dee1b1bb3..d6589b24c68d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -8432,7 +8432,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
/* reset the MAC address in case of policy change */
if (determine_ethernet_addr(tp, &ss) >= 0)
- dev_set_mac_address(tp->netdev, (struct sockaddr *)&ss, NULL);
+ dev_set_mac_address(tp->netdev, &ss, NULL);
netdev = tp->netdev;
if (!netif_running(netdev))
diff --git a/net/core/dev.c b/net/core/dev.c
index f8c8aad7df2e..1f1900ec26b2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9683,6 +9683,7 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
DECLARE_RWSEM(dev_addr_sem);
+/* "sa" is a true struct sockaddr with limited "sa_data" member. */
int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name)
{
size_t size = sizeof(sa->sa_data_min);
diff --git a/net/core/dev_api.c b/net/core/dev_api.c
index b5f293e637d9..6011a5ef649d 100644
--- a/net/core/dev_api.c
+++ b/net/core/dev_api.c
@@ -319,20 +319,20 @@ EXPORT_SYMBOL(dev_set_allmulti);
/**
* dev_set_mac_address() - change Media Access Control Address
* @dev: device
- * @sa: new address
+ * @ss: new address
* @extack: netlink extended ack
*
* Change the hardware (MAC) address of the device
*
* Return: 0 on success, -errno on failure.
*/
-int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
+int dev_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
struct netlink_ext_ack *extack)
{
int ret;
netdev_lock_ops(dev);
- ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
+ ret = netif_set_mac_address(dev, ss, extack);
netdev_unlock_ops(dev);
return ret;
diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
index ee2b190e8e0d..4c07a475c567 100644
--- a/net/ieee802154/nl-phy.c
+++ b/net/ieee802154/nl-phy.c
@@ -234,7 +234,7 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
* dev_set_mac_address require RTNL_LOCK
*/
rtnl_lock();
- rc = dev_set_mac_address(dev, (struct sockaddr *)&addr, NULL);
+ rc = dev_set_mac_address(dev, &addr, NULL);
rtnl_unlock();
if (rc)
goto dev_unregister;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0202db2aea3e..b36947063783 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1058,7 +1058,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_config_apply_mac:
rtnl_lock();
- ret = dev_set_mac_address(dev, (struct sockaddr *)&ndp->pending_mac, NULL);
+ ret = dev_set_mac_address(dev, &ndp->pending_mac, NULL);
rtnl_unlock();
if (ret < 0)
netdev_warn(dev, "NCSI: 'Writing MAC address to device failed\n");
--
2.34.1
^ permalink raw reply related
* [PATCH net-next v2 5/8] net: usb: r8152: Convert to use struct sockaddr_storage internally
From: Kees Cook @ 2025-05-21 20:46 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Kees Cook, Gustavo A . R . Silva, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Hayes Wang,
Douglas Anderson, Grant Grundler, linux-usb, netdev,
Willem de Bruijn, Martin K. Petersen, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Simon Horman, Dr. David Alan Gilbert, Stanislav Fomichev,
Cosmin Ratiu, Lei Yang, Ido Schimmel, Samuel Mendoza-Jonas,
Paul Fertser, Alexander Aring, Stefan Schmidt, Miquel Raynal,
Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Jiri Pirko, Jason Wang, Vladimir Oltean,
Florian Fainelli, Kory Maincent, Maxim Georgiev,
Aleksander Jan Bajkowski, Philipp Hahn, Eric Biggers,
Ard Biesheuvel, Al Viro, Ahmed Zaki, Alexander Lobakin,
Xiao Liang, linux-kernel, linux-nvme, linux-scsi, target-devel,
linux-wpan, linux-hyperv, linux-hardening
In-Reply-To: <20250521204310.it.500-kees@kernel.org>
To support coming API type changes, switch to sockaddr_storage usage
internally.
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hayes Wang <hayeswang@realtek.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Grant Grundler <grundler@chromium.org>
Cc: <linux-usb@vger.kernel.org>
Cc: <netdev@vger.kernel.org>
---
drivers/net/usb/r8152.c | 52 +++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 67f5d30ffcba..b18dee1b1bb3 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1665,14 +1665,14 @@ static int
rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
u32 advertising);
-static int __rtl8152_set_mac_address(struct net_device *netdev, void *p,
+static int __rtl8152_set_mac_address(struct net_device *netdev,
+ struct sockaddr_storage *addr,
bool in_resume)
{
struct r8152 *tp = netdev_priv(netdev);
- struct sockaddr *addr = p;
int ret = -EADDRNOTAVAIL;
- if (!is_valid_ether_addr(addr->sa_data))
+ if (!is_valid_ether_addr(addr->__data))
goto out1;
if (!in_resume) {
@@ -1683,10 +1683,10 @@ static int __rtl8152_set_mac_address(struct net_device *netdev, void *p,
mutex_lock(&tp->control);
- eth_hw_addr_set(netdev, addr->sa_data);
+ eth_hw_addr_set(netdev, addr->__data);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
- pla_ocp_write(tp, PLA_IDR, BYTE_EN_SIX_BYTES, 8, addr->sa_data);
+ pla_ocp_write(tp, PLA_IDR, BYTE_EN_SIX_BYTES, 8, addr->__data);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
mutex_unlock(&tp->control);
@@ -1706,7 +1706,8 @@ static int rtl8152_set_mac_address(struct net_device *netdev, void *p)
* host system provided MAC address.
* Examples of this are Dell TB15 and Dell WD15 docks
*/
-static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
+static int vendor_mac_passthru_addr_read(struct r8152 *tp,
+ struct sockaddr_storage *ss)
{
acpi_status status;
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -1774,47 +1775,48 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
ret = -EINVAL;
goto amacout;
}
- memcpy(sa->sa_data, buf, 6);
+ memcpy(ss->__data, buf, 6);
tp->netdev->addr_assign_type = NET_ADDR_STOLEN;
netif_info(tp, probe, tp->netdev,
- "Using pass-thru MAC addr %pM\n", sa->sa_data);
+ "Using pass-thru MAC addr %pM\n", ss->__data);
amacout:
kfree(obj);
return ret;
}
-static int determine_ethernet_addr(struct r8152 *tp, struct sockaddr *sa)
+static int determine_ethernet_addr(struct r8152 *tp,
+ struct sockaddr_storage *ss)
{
struct net_device *dev = tp->netdev;
int ret;
- sa->sa_family = dev->type;
+ ss->ss_family = dev->type;
- ret = eth_platform_get_mac_address(&tp->udev->dev, sa->sa_data);
+ ret = eth_platform_get_mac_address(&tp->udev->dev, ss->__data);
if (ret < 0) {
if (tp->version == RTL_VER_01) {
- ret = pla_ocp_read(tp, PLA_IDR, 8, sa->sa_data);
+ ret = pla_ocp_read(tp, PLA_IDR, 8, ss->__data);
} else {
/* if device doesn't support MAC pass through this will
* be expected to be non-zero
*/
- ret = vendor_mac_passthru_addr_read(tp, sa);
+ ret = vendor_mac_passthru_addr_read(tp, ss);
if (ret < 0)
ret = pla_ocp_read(tp, PLA_BACKUP, 8,
- sa->sa_data);
+ ss->__data);
}
}
if (ret < 0) {
netif_err(tp, probe, dev, "Get ether addr fail\n");
- } else if (!is_valid_ether_addr(sa->sa_data)) {
+ } else if (!is_valid_ether_addr(ss->__data)) {
netif_err(tp, probe, dev, "Invalid ether addr %pM\n",
- sa->sa_data);
+ ss->__data);
eth_hw_addr_random(dev);
- ether_addr_copy(sa->sa_data, dev->dev_addr);
+ ether_addr_copy(ss->__data, dev->dev_addr);
netif_info(tp, probe, dev, "Random ether addr %pM\n",
- sa->sa_data);
+ ss->__data);
return 0;
}
@@ -1824,17 +1826,17 @@ static int determine_ethernet_addr(struct r8152 *tp, struct sockaddr *sa)
static int set_ethernet_addr(struct r8152 *tp, bool in_resume)
{
struct net_device *dev = tp->netdev;
- struct sockaddr sa;
+ struct sockaddr_storage ss;
int ret;
- ret = determine_ethernet_addr(tp, &sa);
+ ret = determine_ethernet_addr(tp, &ss);
if (ret < 0)
return ret;
if (tp->version == RTL_VER_01)
- eth_hw_addr_set(dev, sa.sa_data);
+ eth_hw_addr_set(dev, ss.__data);
else
- ret = __rtl8152_set_mac_address(dev, &sa, in_resume);
+ ret = __rtl8152_set_mac_address(dev, &ss, in_resume);
return ret;
}
@@ -8421,7 +8423,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
{
struct r8152 *tp = usb_get_intfdata(intf);
struct net_device *netdev;
- struct sockaddr sa;
+ struct sockaddr_storage ss;
if (!tp || !test_bit(PROBED_WITH_NO_ERRORS, &tp->flags))
goto exit;
@@ -8429,8 +8431,8 @@ static int rtl8152_post_reset(struct usb_interface *intf)
rtl_set_accessible(tp);
/* reset the MAC address in case of policy change */
- if (determine_ethernet_addr(tp, &sa) >= 0)
- dev_set_mac_address (tp->netdev, &sa, NULL);
+ if (determine_ethernet_addr(tp, &ss) >= 0)
+ dev_set_mac_address(tp->netdev, (struct sockaddr *)&ss, NULL);
netdev = tp->netdev;
if (!netif_running(netdev))
--
2.34.1
^ permalink raw reply related
* [PATCH net-next v2 2/8] net: core: Switch netif_set_mac_address() to struct sockaddr_storage
From: Kees Cook @ 2025-05-21 20:46 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Kees Cook, Gustavo A . R . Silva, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Andrew Lunn,
Stanislav Fomichev, Cosmin Ratiu, Lei Yang, Ido Schimmel, netdev,
Willem de Bruijn, Martin K. Petersen, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Samuel Mendoza-Jonas, Paul Fertser,
Alexander Aring, Stefan Schmidt, Miquel Raynal, Hayes Wang,
Douglas Anderson, Grant Grundler, Jay Vosburgh, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Jiri Pirko, Jason Wang,
Vladimir Oltean, Florian Fainelli, Kory Maincent, Maxim Georgiev,
Aleksander Jan Bajkowski, Philipp Hahn, Eric Biggers,
Ard Biesheuvel, Al Viro, Ahmed Zaki, Alexander Lobakin,
Xiao Liang, linux-kernel, linux-nvme, linux-scsi, target-devel,
linux-wpan, linux-usb, linux-hyperv, linux-hardening
In-Reply-To: <20250521204310.it.500-kees@kernel.org>
In order to avoid passing around struct sockaddr that has a size the
compiler cannot reason about (nor track at runtime), convert
netif_set_mac_address() to take struct sockaddr_storage. This is just a
cast conversion, so there is are no binary changes. Following patches
will make actual allocation changes.
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Cosmin Ratiu <cratiu@nvidia.com>
Cc: Lei Yang <leiyang@redhat.com>
Cc: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: <netdev@vger.kernel.org>
---
include/linux/netdevice.h | 2 +-
net/core/dev.c | 10 +++++-----
net/core/dev_api.c | 4 ++--
net/core/rtnetlink.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ea9d335de130..47200a394a02 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4212,7 +4212,7 @@ int netif_set_mtu(struct net_device *dev, int new_mtu);
int dev_set_mtu(struct net_device *, int);
int dev_pre_changeaddr_notify(struct net_device *dev, const char *addr,
struct netlink_ext_ack *extack);
-int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa,
+int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
struct netlink_ext_ack *extack);
int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
struct netlink_ext_ack *extack);
diff --git a/net/core/dev.c b/net/core/dev.c
index fccf2167b235..f8c8aad7df2e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9655,7 +9655,7 @@ int dev_pre_changeaddr_notify(struct net_device *dev, const char *addr,
}
EXPORT_SYMBOL(dev_pre_changeaddr_notify);
-int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa,
+int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
struct netlink_ext_ack *extack)
{
const struct net_device_ops *ops = dev->netdev_ops;
@@ -9663,15 +9663,15 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa,
if (!ops->ndo_set_mac_address)
return -EOPNOTSUPP;
- if (sa->sa_family != dev->type)
+ if (ss->ss_family != dev->type)
return -EINVAL;
if (!netif_device_present(dev))
return -ENODEV;
- err = dev_pre_changeaddr_notify(dev, sa->sa_data, extack);
+ err = dev_pre_changeaddr_notify(dev, ss->__data, extack);
if (err)
return err;
- if (memcmp(dev->dev_addr, sa->sa_data, dev->addr_len)) {
- err = ops->ndo_set_mac_address(dev, sa);
+ if (memcmp(dev->dev_addr, ss->__data, dev->addr_len)) {
+ err = ops->ndo_set_mac_address(dev, ss);
if (err)
return err;
}
diff --git a/net/core/dev_api.c b/net/core/dev_api.c
index f9a160ab596f..b5f293e637d9 100644
--- a/net/core/dev_api.c
+++ b/net/core/dev_api.c
@@ -91,7 +91,7 @@ int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
down_write(&dev_addr_sem);
netdev_lock_ops(dev);
- ret = netif_set_mac_address(dev, sa, extack);
+ ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
netdev_unlock_ops(dev);
up_write(&dev_addr_sem);
@@ -332,7 +332,7 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
int ret;
netdev_lock_ops(dev);
- ret = netif_set_mac_address(dev, sa, extack);
+ ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
netdev_unlock_ops(dev);
return ret;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 8a914b37ef6e..9743f1c2ae3c 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3100,7 +3100,7 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
memcpy(sa->sa_data, nla_data(tb[IFLA_ADDRESS]),
dev->addr_len);
- err = netif_set_mac_address(dev, sa, extack);
+ err = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
kfree(sa);
if (err) {
up_write(&dev_addr_sem);
--
2.34.1
^ permalink raw reply related
* [PATCH net-next v2 1/8] net: core: Convert inet_addr_is_any() to sockaddr_storage
From: Kees Cook @ 2025-05-21 20:46 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Kees Cook, Martin K . Petersen, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Mike Christie, Max Gurtovoy, Maurizio Lombardi,
Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy, Simon Horman,
Dr. David Alan Gilbert, linux-nvme, linux-scsi, target-devel,
netdev, Willem de Bruijn, Gustavo A. R. Silva, Andrew Lunn,
Stanislav Fomichev, Cosmin Ratiu, Lei Yang, Ido Schimmel,
Samuel Mendoza-Jonas, Paul Fertser, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Jason Wang, Vladimir Oltean,
Florian Fainelli, Kory Maincent, Maxim Georgiev,
Aleksander Jan Bajkowski, Philipp Hahn, Eric Biggers,
Ard Biesheuvel, Al Viro, Ahmed Zaki, Alexander Lobakin,
Xiao Liang, linux-kernel, linux-wpan, linux-usb, linux-hyperv,
linux-hardening
In-Reply-To: <20250521204310.it.500-kees@kernel.org>
All the callers of inet_addr_is_any() have a sockaddr_storage-backed
sockaddr. Avoid casts and switch prototype to the actual object being
used.
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> # SCSI
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Mike Christie <michael.christie@oracle.com>
Cc: Max Gurtovoy <mgurtovoy@nvidia.com>
Cc: Maurizio Lombardi <mlombard@redhat.com>
Cc: Dmitry Bogdanov <d.bogdanov@yadro.com>
Cc: Mingzhe Zou <mingzhe.zou@easystack.cn>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Simon Horman <horms@kernel.org>
Cc: "Dr. David Alan Gilbert" <linux@treblig.org>
Cc: linux-nvme@lists.infradead.org
Cc: linux-scsi@vger.kernel.org
Cc: target-devel@vger.kernel.org
Cc: netdev@vger.kernel.org
---
include/linux/inet.h | 2 +-
drivers/nvme/target/rdma.c | 2 +-
drivers/nvme/target/tcp.c | 2 +-
drivers/target/iscsi/iscsi_target.c | 2 +-
net/core/utils.c | 8 ++++----
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/linux/inet.h b/include/linux/inet.h
index bd8276e96e60..9158772f3559 100644
--- a/include/linux/inet.h
+++ b/include/linux/inet.h
@@ -55,6 +55,6 @@ extern int in6_pton(const char *src, int srclen, u8 *dst, int delim, const char
extern int inet_pton_with_scope(struct net *net, unsigned short af,
const char *src, const char *port, struct sockaddr_storage *addr);
-extern bool inet_addr_is_any(struct sockaddr *addr);
+bool inet_addr_is_any(struct sockaddr_storage *addr);
#endif /* _LINUX_INET_H */
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 2a4536ef6184..79a5aad2e9d0 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1999,7 +1999,7 @@ static void nvmet_rdma_disc_port_addr(struct nvmet_req *req,
struct nvmet_rdma_port *port = nport->priv;
struct rdma_cm_id *cm_id = port->cm_id;
- if (inet_addr_is_any((struct sockaddr *)&cm_id->route.addr.src_addr)) {
+ if (inet_addr_is_any(&cm_id->route.addr.src_addr)) {
struct nvmet_rdma_rsp *rsp =
container_of(req, struct nvmet_rdma_rsp, req);
struct rdma_cm_id *req_cm_id = rsp->queue->cm_id;
diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 12a5cb8641ca..5cd1cf74f8ff 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -2194,7 +2194,7 @@ static void nvmet_tcp_disc_port_addr(struct nvmet_req *req,
{
struct nvmet_tcp_port *port = nport->priv;
- if (inet_addr_is_any((struct sockaddr *)&port->addr)) {
+ if (inet_addr_is_any(&port->addr)) {
struct nvmet_tcp_cmd *cmd =
container_of(req, struct nvmet_tcp_cmd, req);
struct nvmet_tcp_queue *queue = cmd->queue;
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 620ba6e0ab07..a2dde08c8a62 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3419,7 +3419,7 @@ iscsit_build_sendtargets_response(struct iscsit_cmd *cmd,
}
}
- if (inet_addr_is_any((struct sockaddr *)&np->np_sockaddr))
+ if (inet_addr_is_any(&np->np_sockaddr))
sockaddr = &conn->local_sockaddr;
else
sockaddr = &np->np_sockaddr;
diff --git a/net/core/utils.c b/net/core/utils.c
index 27f4cffaae05..e47feeaa5a49 100644
--- a/net/core/utils.c
+++ b/net/core/utils.c
@@ -399,9 +399,9 @@ int inet_pton_with_scope(struct net *net, __kernel_sa_family_t af,
}
EXPORT_SYMBOL(inet_pton_with_scope);
-bool inet_addr_is_any(struct sockaddr *addr)
+bool inet_addr_is_any(struct sockaddr_storage *addr)
{
- if (addr->sa_family == AF_INET6) {
+ if (addr->ss_family == AF_INET6) {
struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)addr;
const struct sockaddr_in6 in6_any =
{ .sin6_addr = IN6ADDR_ANY_INIT };
@@ -409,13 +409,13 @@ bool inet_addr_is_any(struct sockaddr *addr)
if (!memcmp(in6->sin6_addr.s6_addr,
in6_any.sin6_addr.s6_addr, 16))
return true;
- } else if (addr->sa_family == AF_INET) {
+ } else if (addr->ss_family == AF_INET) {
struct sockaddr_in *in = (struct sockaddr_in *)addr;
if (in->sin_addr.s_addr == htonl(INADDR_ANY))
return true;
} else {
- pr_warn("unexpected address family %u\n", addr->sa_family);
+ pr_warn("unexpected address family %u\n", addr->ss_family);
}
return false;
--
2.34.1
^ permalink raw reply related
* [PATCH net-next v2 0/8] net: Convert dev_set_mac_address() to struct sockaddr_storage
From: Kees Cook @ 2025-05-21 20:46 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Kees Cook, Willem de Bruijn, Martin K. Petersen,
Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Mike Christie, Max Gurtovoy, Maurizio Lombardi, Dmitry Bogdanov,
Mingzhe Zou, Christophe Leroy, Simon Horman,
Dr. David Alan Gilbert, Gustavo A. R. Silva, Andrew Lunn,
Stanislav Fomichev, Cosmin Ratiu, Lei Yang, Ido Schimmel,
Samuel Mendoza-Jonas, Paul Fertser, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Jason Wang, Vladimir Oltean,
Florian Fainelli, Kory Maincent, Maxim Georgiev,
Aleksander Jan Bajkowski, Philipp Hahn, Eric Biggers,
Ard Biesheuvel, Al Viro, Ahmed Zaki, Alexander Lobakin,
Xiao Liang, linux-kernel, linux-nvme, linux-scsi, target-devel,
netdev, linux-wpan, linux-usb, linux-hyperv, linux-hardening
v2:
- add conversion of dev_set_mac_address_user() (kuniyu)
- fix missed sockaddr/sockaddr_storage conversion (kuba)
v1: https://lore.kernel.org/all/20250520222452.work.063-kees@kernel.org/
Hi,
As part of the effort to allow the compiler to reason about object sizes,
we need to deal with the problematic variably sized struct sockaddr,
which has no internal runtime size tracking. In much of the network
stack the use of struct sockaddr_storage has been adopted. Continue the
transition toward this for more of the internal APIs. Specifically:
- inet_addr_is_any()
- netif_set_mac_address()
- dev_set_mac_address()
- dev_set_mac_address_user()
Only a few callers of dev_set_mac_address() needed adjustment; all others
were already using struct sockaddr_storage internally.
-Kees
Kees Cook (8):
net: core: Convert inet_addr_is_any() to sockaddr_storage
net: core: Switch netif_set_mac_address() to struct sockaddr_storage
net/ncsi: Use struct sockaddr_storage for pending_mac
ieee802154: Use struct sockaddr_storage with dev_set_mac_address()
net: usb: r8152: Convert to use struct sockaddr_storage internally
net: core: Convert dev_set_mac_address() to struct sockaddr_storage
rtnetlink: do_setlink: Use struct sockaddr_storage
net: core: Convert dev_set_mac_address_user() to use struct
sockaddr_storage
include/linux/inet.h | 2 +-
include/linux/netdevice.h | 6 ++--
net/ncsi/internal.h | 2 +-
drivers/net/bonding/bond_alb.c | 8 ++---
drivers/net/bonding/bond_main.c | 15 ++++-----
drivers/net/hyperv/netvsc_drv.c | 6 ++--
drivers/net/macvlan.c | 18 +++++-----
drivers/net/tap.c | 14 +++++---
drivers/net/team/team_core.c | 2 +-
drivers/net/tun.c | 8 ++++-
drivers/net/usb/r8152.c | 52 +++++++++++++++--------------
drivers/nvme/target/rdma.c | 2 +-
drivers/nvme/target/tcp.c | 2 +-
drivers/target/iscsi/iscsi_target.c | 2 +-
net/core/dev.c | 11 +++---
net/core/dev_api.c | 11 +++---
net/core/dev_ioctl.c | 6 ++--
net/core/rtnetlink.c | 19 +++--------
net/core/utils.c | 8 ++---
net/ieee802154/nl-phy.c | 6 ++--
net/ncsi/ncsi-rsp.c | 18 +++++-----
21 files changed, 109 insertions(+), 109 deletions(-)
--
2.34.1
^ permalink raw reply
* RE: [EXTERNAL] Re: [PATCH net-next,v2] net: mana: Add support for Multi Vports on Bare metal
From: Haiyang Zhang @ 2025-05-21 17:28 UTC (permalink / raw)
To: Simon Horman
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org, Dexuan Cui,
stephen@networkplumber.org, KY Srinivasan, Paul Rosswurm,
olaf@aepfle.de, vkuznets@redhat.com, davem@davemloft.net,
wei.liu@kernel.org, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, leon@kernel.org, Long Li,
ssengar@linux.microsoft.com, linux-rdma@vger.kernel.org,
daniel@iogearbox.net, john.fastabend@gmail.com,
bpf@vger.kernel.org, ast@kernel.org, hawk@kernel.org,
tglx@linutronix.de, shradhagupta@linux.microsoft.com,
andrew+netdev@lunn.ch, Konstantin Taranov,
linux-kernel@vger.kernel.org
In-Reply-To: <20250521140231.GW365796@horms.kernel.org>
> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Wednesday, May 21, 2025 10:03 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Dexuan Cui
> <decui@microsoft.com>; stephen@networkplumber.org; KY Srinivasan
> <kys@microsoft.com>; Paul Rosswurm <paulros@microsoft.com>;
> olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net;
> wei.liu@kernel.org; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; leon@kernel.org; Long Li <longli@microsoft.com>;
> ssengar@linux.microsoft.com; linux-rdma@vger.kernel.org;
> daniel@iogearbox.net; john.fastabend@gmail.com; bpf@vger.kernel.org;
> ast@kernel.org; hawk@kernel.org; tglx@linutronix.de;
> shradhagupta@linux.microsoft.com; andrew+netdev@lunn.ch; Konstantin
> Taranov <kotaranov@microsoft.com>; linux-kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH net-next,v2] net: mana: Add support for
> Multi Vports on Bare metal
>
> On Mon, May 19, 2025 at 09:20:36AM -0700, Haiyang Zhang wrote:
> > To support Multi Vports on Bare metal, increase the device config
> response
> > version. And, skip the register HW vport, and register filter steps,
> when
> > the Bare metal hostmode is set.
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > v2:
> > Updated comments as suggested by ALOK TIWARI.
> > Fixed the version check.
> >
> > ---
> > drivers/net/ethernet/microsoft/mana/mana_en.c | 24 ++++++++++++-------
> > include/net/mana/mana.h | 4 +++-
> > 2 files changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index 2bac6be8f6a0..9c58d9e0bbb5 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > @@ -921,7 +921,7 @@ static void mana_pf_deregister_filter(struct
> mana_port_context *apc)
> >
> > static int mana_query_device_cfg(struct mana_context *ac, u32
> proto_major_ver,
> > u32 proto_minor_ver, u32 proto_micro_ver,
> > - u16 *max_num_vports)
> > + u16 *max_num_vports, u8 *bm_hostmode)
> > {
> > struct gdma_context *gc = ac->gdma_dev->gdma_context;
> > struct mana_query_device_cfg_resp resp = {};
> > @@ -932,7 +932,7 @@ static int mana_query_device_cfg(struct mana_context
> *ac, u32 proto_major_ver,
> > mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_DEV_CONFIG,
> > sizeof(req), sizeof(resp));
> >
> > - req.hdr.resp.msg_version = GDMA_MESSAGE_V2;
> > + req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
> >
> > req.proto_major_ver = proto_major_ver;
> > req.proto_minor_ver = proto_minor_ver;
>
> > @@ -956,11 +956,16 @@ static int mana_query_device_cfg(struct
> mana_context *ac, u32 proto_major_ver,
> >
> > *max_num_vports = resp.max_num_vports;
> >
> > - if (resp.hdr.response.msg_version == GDMA_MESSAGE_V2)
> > + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2)
> > gc->adapter_mtu = resp.adapter_mtu;
> > else
> > gc->adapter_mtu = ETH_FRAME_LEN;
> >
> > + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3)
> > + *bm_hostmode = resp.bm_hostmode;
> > + else
> > + *bm_hostmode = 0;
>
> Hi,
>
> Perhaps not strictly related to this patch, but I see
> that mana_verify_resp_hdr() is called a few lines above.
> And that verifies a minimum msg_version. But I do not see
> any verification of the maximum msg_version supported by the code.
>
> I am concerned about a hypothetical scenario where, say the as yet unknown
> version 5 is sent as the version, and the above behaviour is used, while
> not being correct.
>
> Could you shed some light on this?
>
In driver, we specify the expected reply msg version is v3 here:
req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
If the HW side is upgraded, it won't send reply msg version higher
than expected, which may break the driver.
Thanks,
- Haiyang
^ permalink raw reply
* RE: [PATCH v3 4/4] net: mana: Allocate MSI-X vectors dynamically
From: Michael Kelley @ 2025-05-21 16:27 UTC (permalink / raw)
To: Shradha Gupta, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Simon Horman,
Leon Romanovsky, Maxim Levitsky, Erni Sri Satya Vennela,
Peter Zijlstra
Cc: linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy�~Dski,
Lorenzo Pieralisi, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Paul Rosswurm, Shradha Gupta
In-Reply-To: <1746785637-4881-1-git-send-email-shradhagupta@linux.microsoft.com>
From: Shradha Gupta <shradhagupta@linux.microsoft.com> Sent: Friday, May 9, 2025 3:14 AM
>
> Currently, the MANA driver allocates MSI-X vectors statically based on
> MANA_MAX_NUM_QUEUES and num_online_cpus() values and in some cases ends
> up allocating more vectors than it needs. This is because, by this time
> we do not have a HW channel and do not know how many IRQs should be
> allocated.
>
> To avoid this, we allocate 1 MSI-X vector during the creation of HWC and
> after getting the value supported by hardware, dynamically add the
> remaining MSI-X vectors.
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> Changes in v3:
> * implemented irq_contexts as xarrays rather than list
> * split the patch to create a perparation patch around irq_setup()
> * add log when IRQ allocation/setup for remaining IRQs fails
> ---
> Changes in v2:
> * Use string 'MSI-X vectors' instead of 'pci vectors'
> * make skip-cpu a bool instead of int
> * rearrange the comment arout skip_cpu variable appropriately
> * update the capability bit for driver indicating dynamic IRQ allocation
> * enforced max line length to 80
> * enforced RCT convention
> * initialized gic to NULL, for when there is a possibility of gic
> not being populated correctly
> ---
> .../net/ethernet/microsoft/mana/gdma_main.c | 248 +++++++++++++++---
> include/net/mana/gdma.h | 8 +-
> 2 files changed, 211 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 2de42ce43373..f07cebffc30d 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -6,6 +6,8 @@
> #include <linux/pci.h>
> #include <linux/utsname.h>
> #include <linux/version.h>
> +#include <linux/msi.h>
> +#include <linux/irqdomain.h>
>
> #include <net/mana/mana.h>
>
> @@ -80,8 +82,15 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> return err ? err : -EPROTO;
> }
>
> - if (gc->num_msix_usable > resp.max_msix)
> - gc->num_msix_usable = resp.max_msix;
> + if (!pci_msix_can_alloc_dyn(pdev)) {
> + if (gc->num_msix_usable > resp.max_msix)
> + gc->num_msix_usable = resp.max_msix;
> + } else {
> + /* If dynamic allocation is enabled we have already allocated
> + * hwc msi
> + */
> + gc->num_msix_usable = min(resp.max_msix, num_online_cpus() + 1);
> + }
>
> if (gc->num_msix_usable <= 1)
> return -ENOSPC;
> @@ -482,7 +491,9 @@ static int mana_gd_register_irq(struct gdma_queue *queue,
> }
>
> queue->eq.msix_index = msi_index;
> - gic = &gc->irq_contexts[msi_index];
> + gic = xa_load(&gc->irq_contexts, msi_index);
> + if (!gic)
> + return -EINVAL;
>
> spin_lock_irqsave(&gic->lock, flags);
> list_add_rcu(&queue->entry, &gic->eq_list);
> @@ -507,7 +518,10 @@ static void mana_gd_deregiser_irq(struct gdma_queue *queue)
> if (WARN_ON(msix_index >= gc->num_msix_usable))
> return;
>
> - gic = &gc->irq_contexts[msix_index];
> + gic = xa_load(&gc->irq_contexts, msix_index);
> + if (!gic)
> + return;
If xa_load() doesn't return a valid gic, it seems like that would warrant a
WARN_ON(), like the above case where the msix_index is out of range.
> +
> spin_lock_irqsave(&gic->lock, flags);
> list_for_each_entry_rcu(eq, &gic->eq_list, entry) {
> if (queue == eq) {
> @@ -1329,29 +1343,96 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
> return 0;
> }
>
> -static int mana_gd_setup_irqs(struct pci_dev *pdev)
> +static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> {
> struct gdma_context *gc = pci_get_drvdata(pdev);
> - unsigned int max_queues_per_port;
> struct gdma_irq_context *gic;
> - unsigned int max_irqs, cpu;
> - int start_irq_index = 1;
> - int nvec, *irqs, irq;
> + bool skip_first_cpu = false;
> int err, i = 0, j;
Initializing "i" to 0 is superfluous. The "for" loop below does it.
> + int *irqs, irq;
>
> cpus_read_lock();
> - max_queues_per_port = num_online_cpus();
> - if (max_queues_per_port > MANA_MAX_NUM_QUEUES)
> - max_queues_per_port = MANA_MAX_NUM_QUEUES;
>
> - /* Need 1 interrupt for the Hardware communication Channel (HWC) */
> - max_irqs = max_queues_per_port + 1;
> + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> + if (!irqs) {
> + err = -ENOMEM;
> + goto free_irq_vector;
> + }
>
> - nvec = pci_alloc_irq_vectors(pdev, 2, max_irqs, PCI_IRQ_MSIX);
> - if (nvec < 0) {
> - cpus_read_unlock();
> - return nvec;
> + for (i = 0; i < nvec; i++) {
> + gic = kcalloc(1, sizeof(struct gdma_irq_context), GFP_KERNEL);
kcalloc() with a constant 1 first argument is a bit unusual. Just use kzalloc() since
there's no array here?
> + if (!gic) {
> + err = -ENOMEM;
> + goto free_irq;
> + }
> + gic->handler = mana_gd_process_eq_events;
> + INIT_LIST_HEAD(&gic->eq_list);
> + spin_lock_init(&gic->lock);
> +
> + snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> + i, pci_name(pdev));
> +
> + /* one pci vector is already allocated for HWC */
> + irqs[i] = pci_irq_vector(pdev, i + 1);
> + if (irqs[i] < 0) {
> + err = irqs[i];
> + goto free_current_gic;
> + }
> +
> + err = request_irq(irqs[i], mana_gd_intr, 0, gic->name, gic);
> + if (err)
> + goto free_current_gic;
> +
> + xa_store(&gc->irq_contexts, i + 1, gic, GFP_KERNEL);
> }
> +
> + /*
> + * When calling irq_setup() for dynamically added IRQs, if number of
> + * CPUs is more than or equal to allocated MSI-X, we need to skip the
> + * first CPU sibling group since they are already affinitized to HWC IRQ
> + */
> + if (gc->num_msix_usable <= num_online_cpus())
> + skip_first_cpu = true;
> +
> + err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
> + if (err)
> + goto free_irq;
> +
> + cpus_read_unlock();
> + kfree(irqs);
> + return 0;
> +
> +free_current_gic:
> + kfree(gic);
> +free_irq:
In the error case, this label is reached with "i" in two possible
states. Case 1: It might be the index of the entry that failed due to
the "goto free_current_gic" statements. Case 2: It might be the
index of one entry past all the successfully requested irqs, when the
failure occurs on irq_setup() and the code does "goto free_irq".
> + for (j = i; j >= 0; j--) {
So the "for" loop starts with "j" set to an index that doesn't
exist (in Case 2 above), or an index that is only partially
complete (Case 1 above).
And actually, local variable "j" isn't needed for this loop.
It could just count down using "i".
> + irq = pci_irq_vector(pdev, j);
This seems to be looking up the wrong irq vector. In the main
loop earlier, the index to pci_irq_vector() is "i + 1" but there's
no "+ 1" here.
> + gic = xa_load(&gc->irq_contexts, j);
> + if (!gic)
> + continue;
So evidently it is expected that this xa_load() will fail
the first time through this "j" loop. In Case 1, the xa_store()
was never done, and in Case 2, the index starts out one
too big.
> +
> + irq_update_affinity_hint(irq, NULL);
> + free_irq(irq, gic);
> + xa_erase(&gc->irq_contexts, j);
> + kfree(gic);
> + }
Except for the wrong index to pci_irq_vector(), I think this works,
but it's a bit bizarre. More natural would be to initialize "j" to
"i - 1" so that the first iteration through the loop isn't degenerate.
In that case, all the calls to xa_load() should succeed, and you
might put a WARN_ON() if there's a failure.
> + kfree(irqs);
> +free_irq_vector:
> + cpus_read_unlock();
> + return err;
> +}
> +
> +static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> +{
> + struct gdma_context *gc = pci_get_drvdata(pdev);
> + struct gdma_irq_context *gic;
> + int start_irq_index = 1;
> + unsigned int cpu;
> + int *irqs, irq;
> + int err, i = 0, j;
Initializing "i" to 0 is superfluous.
> +
> + cpus_read_lock();
> +
> if (nvec <= num_online_cpus())
> start_irq_index = 0;
>
> @@ -1361,15 +1442,13 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
> goto free_irq_vector;
> }
>
> - gc->irq_contexts = kcalloc(nvec, sizeof(struct gdma_irq_context),
> - GFP_KERNEL);
> - if (!gc->irq_contexts) {
> - err = -ENOMEM;
> - goto free_irq_array;
> - }
> -
> for (i = 0; i < nvec; i++) {
> - gic = &gc->irq_contexts[i];
> + gic = kcalloc(1, sizeof(struct gdma_irq_context), GFP_KERNEL);
kcalloc() with a constant 1 first argument is a bit unusual. Just use kzalloc() since
there's no array here?
> + if (!gic) {
> + err = -ENOMEM;
> + goto free_irq;
> + }
> +
> gic->handler = mana_gd_process_eq_events;
> INIT_LIST_HEAD(&gic->eq_list);
> spin_lock_init(&gic->lock);
> @@ -1384,13 +1463,13 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
> irq = pci_irq_vector(pdev, i);
> if (irq < 0) {
> err = irq;
> - goto free_irq;
> + goto free_current_gic;
> }
>
> if (!i) {
> err = request_irq(irq, mana_gd_intr, 0, gic->name, gic);
> if (err)
> - goto free_irq;
> + goto free_current_gic;
>
> /* If number of IRQ is one extra than number of online CPUs,
> * then we need to assign IRQ0 (hwc irq) and IRQ1 to
> @@ -1408,39 +1487,110 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
> }
> } else {
> irqs[i - start_irq_index] = irq;
> - err = request_irq(irqs[i - start_irq_index], mana_gd_intr, 0,
> - gic->name, gic);
> + err = request_irq(irqs[i - start_irq_index],
> + mana_gd_intr, 0, gic->name, gic);
> if (err)
> - goto free_irq;
> + goto free_current_gic;
> }
> +
> + xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> }
FWIW, I think all this logic around "start_irq_index" could be simplified,
though I haven't worked through the details. If it would simplify the code,
it would be fine to allocate the "irqs" array with one extra entry that is unused
if IRQ0 and IRQ1 use the same CPUs.
>
> err = irq_setup(irqs, (nvec - start_irq_index), gc->numa_node, false);
> if (err)
> goto free_irq;
>
> - gc->max_num_msix = nvec;
> - gc->num_msix_usable = nvec;
> cpus_read_unlock();
> kfree(irqs);
> return 0;
>
> +free_current_gic:
> + kfree(gic);
> free_irq:
> for (j = i - 1; j >= 0; j--) {
In this case j is initialized to i - 1, which is what I expected in
the previous case.
Again, this loop could just countdown using "i" instead of
introducing "j".
> irq = pci_irq_vector(pdev, j);
> - gic = &gc->irq_contexts[j];
> + gic = xa_load(&gc->irq_contexts, j);
> + if (!gic)
> + continue;
Failure to get a valid gic should never happen, so perhaps a WARN_ON()
is appropriate.
>
> irq_update_affinity_hint(irq, NULL);
> free_irq(irq, gic);
> + xa_erase(&gc->irq_contexts, j);
> + kfree(gic);
> }
>
> - kfree(gc->irq_contexts);
> - gc->irq_contexts = NULL;
> -free_irq_array:
> kfree(irqs);
> free_irq_vector:
> + xa_destroy(&gc->irq_contexts);
This seems like the wrong place to be doing xa_destroy(). It leads
to inconsistencies. For example, if mana_gd_setup_hwc_irqs()
fails, it may have failed before calling mana_gd_setup_irqs(), in
which case the xa_destroy() is not done. Or if the failure occurred here
in mana_gd_setup_irqs(), then xa_destroy() will have been done.
Ideally, the xa_destroy() for error cases could be done in
mana_gd_probe() where the xa_init() is done so that the calls match
up, and of course in mana_gd_remove() when the device goes away
entirely.
> cpus_read_unlock();
> - pci_free_irq_vectors(pdev);
> + return err;
> +}
> +
> +static int mana_gd_setup_hwc_irqs(struct pci_dev *pdev)
> +{
> + struct gdma_context *gc = pci_get_drvdata(pdev);
> + unsigned int max_irqs, min_irqs;
> + int max_queues_per_port;
> + int nvec, err;
> +
> + if (pci_msix_can_alloc_dyn(pdev)) {
> + max_irqs = 1;
> + min_irqs = 1;
> + } else {
> + max_queues_per_port = num_online_cpus();
> + if (max_queues_per_port > MANA_MAX_NUM_QUEUES)
> + max_queues_per_port = MANA_MAX_NUM_QUEUES;
> + /* Need 1 interrupt for HWC */
> + max_irqs = max_queues_per_port + 1;
This code is simply being copied from existing code, but it would be nicer to
code it as:
max_irqs = min(num_online_cpus(), MANA_MAX_NUM_QUEUES) + 1;
Explicitly using the "min" function reduces the cognitive effort to parse
the "if" statement and figure out that it is picking the minimum of the two
values. And the local variable max_queues_per_port can be dropped, but
keep the comment :-)
> + min_irqs = 2;
> + }
> +
> + nvec = pci_alloc_irq_vectors(pdev, min_irqs, max_irqs, PCI_IRQ_MSIX);
> + if (nvec < 0)
> + return nvec;
> +
> + err = mana_gd_setup_irqs(pdev, nvec);
> + if (err) {
> + pci_free_irq_vectors(pdev);
> + return err;
> + }
> +
> + gc->num_msix_usable = nvec;
> + gc->max_num_msix = nvec;
> +
> + return err;
"err" should always be zero at this point, so could do "return 0"
> +}
> +
> +static int mana_gd_setup_remaining_irqs(struct pci_dev *pdev)
> +{
> + struct gdma_context *gc = pci_get_drvdata(pdev);
> + int max_irqs, i, err = 0;
> + struct msi_map irq_map;
> +
> + if (!pci_msix_can_alloc_dyn(pdev))
> + /* remain irqs are already allocated with HWC IRQ */
> + return 0;
> +
> + /* allocate only remaining IRQs*/
> + max_irqs = gc->num_msix_usable - 1;
> +
> + for (i = 1; i <= max_irqs; i++) {
> + irq_map = pci_msix_alloc_irq_at(pdev, i, NULL);
> + if (!irq_map.virq) {
> + err = irq_map.index;
> + /* caller will handle cleaning up all allocated
> + * irqs, after HWC is destroyed
> + */
> + return err;
> + }
> + }
> +
> + err = mana_gd_setup_dyn_irqs(pdev, max_irqs);
> + if (err)
> + return err;
> +
> + gc->max_num_msix = gc->max_num_msix + max_irqs;
> +
> return err;
Again, err must always be zero here.
> }
>
> @@ -1458,19 +1608,22 @@ static void mana_gd_remove_irqs(struct pci_dev *pdev)
> if (irq < 0)
> continue;
>
> - gic = &gc->irq_contexts[i];
> + gic = xa_load(&gc->irq_contexts, i);
> + if (!gic)
> + continue;
>
> /* Need to clear the hint before free_irq */
> irq_update_affinity_hint(irq, NULL);
> free_irq(irq, gic);
> + xa_erase(&gc->irq_contexts, i);
> + kfree(gic);
> }
>
> pci_free_irq_vectors(pdev);
>
> gc->max_num_msix = 0;
> gc->num_msix_usable = 0;
> - kfree(gc->irq_contexts);
> - gc->irq_contexts = NULL;
> + xa_destroy(&gc->irq_contexts);
As noted above, I'm doubtful about this being the right place to do
xa_destroy().
> }
>
> static int mana_gd_setup(struct pci_dev *pdev)
> @@ -1481,9 +1634,10 @@ static int mana_gd_setup(struct pci_dev *pdev)
> mana_gd_init_registers(pdev);
> mana_smc_init(&gc->shm_channel, gc->dev, gc->shm_base);
>
> - err = mana_gd_setup_irqs(pdev);
> + err = mana_gd_setup_hwc_irqs(pdev);
> if (err) {
> - dev_err(gc->dev, "Failed to setup IRQs: %d\n", err);
> + dev_err(gc->dev, "Failed to setup IRQs for HWC creation: %d\n",
> + err);
> return err;
> }
>
> @@ -1499,6 +1653,12 @@ static int mana_gd_setup(struct pci_dev *pdev)
> if (err)
> goto destroy_hwc;
>
> + err = mana_gd_setup_remaining_irqs(pdev);
> + if (err) {
> + dev_err(gc->dev, "Failed to setup remaining IRQs: %d", err);
> + goto destroy_hwc;
> + }
> +
> err = mana_gd_detect_devices(pdev);
> if (err)
> goto destroy_hwc;
> @@ -1575,6 +1735,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> gc->is_pf = mana_is_pf(pdev->device);
> gc->bar0_va = bar0_va;
> gc->dev = &pdev->dev;
> + xa_init(&gc->irq_contexts);
>
> if (gc->is_pf)
> gc->mana_pci_debugfs = debugfs_create_dir("0", mana_debugfs_root);
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index 228603bf03f2..f20d1d1ea5e8 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -373,7 +373,7 @@ struct gdma_context {
> unsigned int max_num_queues;
> unsigned int max_num_msix;
> unsigned int num_msix_usable;
> - struct gdma_irq_context *irq_contexts;
> + struct xarray irq_contexts;
>
> /* L2 MTU */
> u16 adapter_mtu;
> @@ -558,12 +558,16 @@ enum {
> /* Driver can handle holes (zeros) in the device list */
> #define GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP BIT(11)
>
> +/* Driver supports dynamic MSI-X vector allocation */
> +#define GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT BIT(13)
> +
> #define GDMA_DRV_CAP_FLAGS1 \
> (GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
> GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
> GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG | \
> GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT | \
> - GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP)
> + GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP | \
> + GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT)
>
> #define GDMA_DRV_CAP_FLAGS2 0
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH net-next,v2] net: mana: Add support for Multi Vports on Bare metal
From: Simon Horman @ 2025-05-21 14:02 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-hyperv, netdev, decui, stephen, kys, paulros, olaf,
vkuznets, davem, wei.liu, edumazet, kuba, pabeni, leon, longli,
ssengar, linux-rdma, daniel, john.fastabend, bpf, ast, hawk, tglx,
shradhagupta, andrew+netdev, kotaranov, linux-kernel
In-Reply-To: <1747671636-5810-1-git-send-email-haiyangz@microsoft.com>
On Mon, May 19, 2025 at 09:20:36AM -0700, Haiyang Zhang wrote:
> To support Multi Vports on Bare metal, increase the device config response
> version. And, skip the register HW vport, and register filter steps, when
> the Bare metal hostmode is set.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> v2:
> Updated comments as suggested by ALOK TIWARI.
> Fixed the version check.
>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 24 ++++++++++++-------
> include/net/mana/mana.h | 4 +++-
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 2bac6be8f6a0..9c58d9e0bbb5 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -921,7 +921,7 @@ static void mana_pf_deregister_filter(struct mana_port_context *apc)
>
> static int mana_query_device_cfg(struct mana_context *ac, u32 proto_major_ver,
> u32 proto_minor_ver, u32 proto_micro_ver,
> - u16 *max_num_vports)
> + u16 *max_num_vports, u8 *bm_hostmode)
> {
> struct gdma_context *gc = ac->gdma_dev->gdma_context;
> struct mana_query_device_cfg_resp resp = {};
> @@ -932,7 +932,7 @@ static int mana_query_device_cfg(struct mana_context *ac, u32 proto_major_ver,
> mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_DEV_CONFIG,
> sizeof(req), sizeof(resp));
>
> - req.hdr.resp.msg_version = GDMA_MESSAGE_V2;
> + req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
>
> req.proto_major_ver = proto_major_ver;
> req.proto_minor_ver = proto_minor_ver;
> @@ -956,11 +956,16 @@ static int mana_query_device_cfg(struct mana_context *ac, u32 proto_major_ver,
>
> *max_num_vports = resp.max_num_vports;
>
> - if (resp.hdr.response.msg_version == GDMA_MESSAGE_V2)
> + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2)
> gc->adapter_mtu = resp.adapter_mtu;
> else
> gc->adapter_mtu = ETH_FRAME_LEN;
>
> + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3)
> + *bm_hostmode = resp.bm_hostmode;
> + else
> + *bm_hostmode = 0;
Hi,
Perhaps not strictly related to this patch, but I see
that mana_verify_resp_hdr() is called a few lines above.
And that verifies a minimum msg_version. But I do not see
any verification of the maximum msg_version supported by the code.
I am concerned about a hypothetical scenario where, say the as yet unknown
version 5 is sent as the version, and the above behaviour is used, while
not being correct.
Could you shed some light on this?
...
^ permalink raw reply
* Re: [PATCH 6/7] net: core: Convert dev_set_mac_address() to struct sockaddr_storage
From: kernel test robot @ 2025-05-21 13:42 UTC (permalink / raw)
To: Kees Cook, Kuniyuki Iwashima
Cc: oe-kbuild-all, Kees Cook, Jakub Kicinski, Jay Vosburgh,
Andrew Lunn, Eric Dumazet, Paolo Abeni, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Jiri Pirko, Simon Horman,
Alexander Aring, Stefan Schmidt, Miquel Raynal,
Samuel Mendoza-Jonas, Paul Fertser, Hayes Wang, Douglas Anderson,
Grant Grundler, Stanislav Fomichev, Cosmin Ratiu, Lei Yang,
netdev, linux-hyperv, linux-usb, linux-wpan, Christoph Hellwig,
Sagi Grimberg
In-Reply-To: <20250520223108.2672023-6-kees@kernel.org>
Hi Kees,
kernel test robot noticed the following build errors:
[auto build test ERROR on linux-nvme/for-next]
[also build test ERROR on mkp-scsi/for-next kees/for-next/pstore kees/for-next/kspp linus/master v6.15-rc7 next-20250521]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/net-core-Convert-inet_addr_is_any-to-sockaddr_storage/20250521-063445
base: git://git.infradead.org/nvme.git for-next
patch link: https://lore.kernel.org/r/20250520223108.2672023-6-kees%40kernel.org
patch subject: [PATCH 6/7] net: core: Convert dev_set_mac_address() to struct sockaddr_storage
config: arc-randconfig-001-20250521 (https://download.01.org/0day-ci/archive/20250521/202505212149.6QGSFucw-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250521/202505212149.6QGSFucw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505212149.6QGSFucw-lkp@intel.com/
All errors (new ones prefixed by >>):
net/core/dev_api.c: In function 'dev_set_mac_address':
>> net/core/dev_api.c:318:35: error: 'sa' undeclared (first use in this function); did you mean 'ss'?
318 | ret = netif_set_mac_address(dev, sa, extack);
| ^~
| ss
net/core/dev_api.c:318:35: note: each undeclared identifier is reported only once for each function it appears in
vim +318 net/core/dev_api.c
301
302 /**
303 * dev_set_mac_address() - change Media Access Control Address
304 * @dev: device
305 * @ss: new address
306 * @extack: netlink extended ack
307 *
308 * Change the hardware (MAC) address of the device
309 *
310 * Return: 0 on success, -errno on failure.
311 */
312 int dev_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
313 struct netlink_ext_ack *extack)
314 {
315 int ret;
316
317 netdev_lock_ops(dev);
> 318 ret = netif_set_mac_address(dev, sa, extack);
319 netdev_unlock_ops(dev);
320
321 return ret;
322 }
323 EXPORT_SYMBOL(dev_set_mac_address);
324
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH net,v2] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Subbaraya Sundeep @ 2025-05-21 10:51 UTC (permalink / raw)
To: Saurabh Sengar
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf, ssengar, stable
In-Reply-To: <1747823103-3420-1-git-send-email-ssengar@linux.microsoft.com>
On 2025-05-21 at 10:25:03, Saurabh Sengar (ssengar@linux.microsoft.com) wrote:
> The MANA driver's probe registers netdevice via the following call chain:
>
> mana_probe()
> register_netdev()
> register_netdevice()
>
> register_netdevice() calls notifier callback for netvsc driver,
> holding the netdev mutex via netdev_lock_ops().
>
> Further this netvsc notifier callback end up attempting to acquire the
> same lock again in dev_xdp_propagate() leading to deadlock.
>
> netvsc_netdev_event()
> netvsc_vf_setxdp()
> dev_xdp_propagate()
>
> This deadlock was not observed so far because net_shaper_ops was never set,
> and thus the lock was effectively a no-op in this case. Fix this by using
> netif_xdp_propagate() instead of dev_xdp_propagate() to avoid recursive
> locking in this path.
>
> Also, clean up the unregistration path by removing the unnecessary call to
> netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
> performs this cleanup via dev_xdp_uninstall().
>
> Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf")
> Cc: stable@vger.kernel.org
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> Tested-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Subbaraya Sundeep <sbhatta@marvell.com>
Thanks,
Sundeep
> ---
> [V2]
> - Modified commit message
>
> drivers/net/hyperv/netvsc_bpf.c | 2 +-
> drivers/net/hyperv/netvsc_drv.c | 2 --
> net/core/dev.c | 1 +
> 3 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
> index e01c5997a551..1dd3755d9e6d 100644
> --- a/drivers/net/hyperv/netvsc_bpf.c
> +++ b/drivers/net/hyperv/netvsc_bpf.c
> @@ -183,7 +183,7 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
> xdp.command = XDP_SETUP_PROG;
> xdp.prog = prog;
>
> - ret = dev_xdp_propagate(vf_netdev, &xdp);
> + ret = netif_xdp_propagate(vf_netdev, &xdp);
>
> if (ret && prog)
> bpf_prog_put(prog);
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index d8b169ac0343..ee3aaf9c10e6 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -2462,8 +2462,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
>
> netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
>
> - netvsc_vf_setxdp(vf_netdev, NULL);
> -
> reinit_completion(&net_device_ctx->vf_add);
> netdev_rx_handler_unregister(vf_netdev);
> netdev_upper_dev_unlink(vf_netdev, ndev);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index fccf2167b235..8c6c9d7fba26 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9953,6 +9953,7 @@ int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf)
>
> return dev->netdev_ops->ndo_bpf(dev, bpf);
> }
> +EXPORT_SYMBOL_GPL(netif_xdp_propagate);
>
> u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode)
> {
> --
> 2.43.0
>
^ permalink raw reply
* [PATCH net,v2] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Saurabh Sengar @ 2025-05-21 10:25 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf
Cc: ssengar, stable, Saurabh Sengar
The MANA driver's probe registers netdevice via the following call chain:
mana_probe()
register_netdev()
register_netdevice()
register_netdevice() calls notifier callback for netvsc driver,
holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the
same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event()
netvsc_vf_setxdp()
dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never set,
and thus the lock was effectively a no-op in this case. Fix this by using
netif_xdp_propagate() instead of dev_xdp_propagate() to avoid recursive
locking in this path.
Also, clean up the unregistration path by removing the unnecessary call to
netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
performs this cleanup via dev_xdp_uninstall().
Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf")
Cc: stable@vger.kernel.org
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Tested-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
[V2]
- Modified commit message
drivers/net/hyperv/netvsc_bpf.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 2 --
net/core/dev.c | 1 +
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index e01c5997a551..1dd3755d9e6d 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -183,7 +183,7 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
xdp.command = XDP_SETUP_PROG;
xdp.prog = prog;
- ret = dev_xdp_propagate(vf_netdev, &xdp);
+ ret = netif_xdp_propagate(vf_netdev, &xdp);
if (ret && prog)
bpf_prog_put(prog);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index d8b169ac0343..ee3aaf9c10e6 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2462,8 +2462,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
- netvsc_vf_setxdp(vf_netdev, NULL);
-
reinit_completion(&net_device_ctx->vf_add);
netdev_rx_handler_unregister(vf_netdev);
netdev_upper_dev_unlink(vf_netdev, ndev);
diff --git a/net/core/dev.c b/net/core/dev.c
index fccf2167b235..8c6c9d7fba26 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9953,6 +9953,7 @@ int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf)
return dev->netdev_ops->ndo_bpf(dev, bpf);
}
+EXPORT_SYMBOL_GPL(netif_xdp_propagate);
u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode)
{
--
2.43.0
^ permalink raw reply related
* [PATCH] firmware: smccc: support both conduits for getting hyp UUID
From: Anirudh Rayabharam @ 2025-05-21 9:40 UTC (permalink / raw)
To: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla
Cc: anirudh, linux-hyperv, linux-arm-kernel, linux-kernel
From: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
When Linux is running as the root partition under Microsoft Hypervisor
(MSHV) a.k.a Hyper-V, smc is used as the conduit for smc calls.
Extend arm_smccc_hypervisor_has_uuid() to support this usecase. Use
arm_smccc_1_1_invoke to retrieve and use the appropriate conduit instead
of supporting only hvc.
Boot tested on MSHV guest, MSHV root & KVM guest.
Signed-off-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
---
drivers/firmware/smccc/smccc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index cd65b434dc6e..bdee057db2fd 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -72,10 +72,7 @@ bool arm_smccc_hypervisor_has_uuid(const uuid_t *hyp_uuid)
struct arm_smccc_res res = {};
uuid_t uuid;
- if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
- return false;
-
- arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
+ arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
return false;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-05-21 9:32 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Roman Kisel, Anirudh Rayabharam, Saurabh Sengar, Nuno Das Neves,
ALOK TIWARI, linux-kernel, linux-hyperv
In-Reply-To: <80853cdb-fd34-4a5e-99a0-1a71b8ce8226@linux.microsoft.com>
On 5/21/2025 11:33 AM, Naman Jain wrote:
>
>
> On 5/21/2025 12:25 AM, Stanislav Kinsburskii wrote:
>> On Mon, May 19, 2025 at 10:26:42AM +0530, Naman Jain wrote:
>>> Provide an interface for Virtual Machine Monitor like OpenVMM and its
>>> use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
>>> Expose devices and support IOCTLs for features like VTL creation,
>>> VTL0 memory management, context switch, making hypercalls,
>>> mapping VTL0 address space to VTL2 userspace, getting new VMBus
>>> messages and channel events in VTL2 etc.
>>>
>>> Co-developed-by: Roman Kisel <romank@linux.microsoft.com>
>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>> Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>> Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
>>> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
>>> Message-ID: <20250512140432.2387503-3-namjain@linux.microsoft.com>
>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>>> ---
>>> drivers/hv/Kconfig | 20 +
>>> drivers/hv/Makefile | 7 +-
>>> drivers/hv/mshv_vtl.h | 52 +
>>> drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
>>> include/hyperv/hvgdk_mini.h | 81 ++
>>> include/hyperv/hvhdk.h | 1 +
>>> include/uapi/linux/mshv.h | 82 ++
>>> 7 files changed, 2025 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/hv/mshv_vtl.h
>>> create mode 100644 drivers/hv/mshv_vtl_main.c
>>>
>>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
>>> index eefa0b559b73..21cee5564d70 100644
>>> --- a/drivers/hv/Kconfig
>>> +++ b/drivers/hv/Kconfig
>>> @@ -72,4 +72,24 @@ config MSHV_ROOT
>>> If unsure, say N.
>>> +config MSHV_VTL
>>> + tristate "Microsoft Hyper-V VTL driver"
>>> + depends on HYPERV && X86_64
>>> + depends on TRANSPARENT_HUGEPAGE
>>
>> Why does it depend on TRANSPARENT_HUGEPAGE?
>>
>
> Thanks for reviewing. This config is required for below functions which
> are used for mshv_vtl_low device.
>
> vm_fault_t mshv_vtl_low_huge_fault ->
> * vmf_insert_pfn_pmd
> * vmf_insert_pfn_pud
>
>
>> <snip>
>>
>>> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
>>> index 1be7f6a02304..cc11000e39f4 100644
>>> --- a/include/hyperv/hvgdk_mini.h
>>> +++ b/include/hyperv/hvgdk_mini.h
>>> @@ -882,6 +882,23 @@ struct hv_get_vp_from_apic_id_in {
>>> u32 apic_ids[];
>>> } __packed;
>>> +union hv_register_vsm_partition_config {
>>> + __u64 as_u64;
>>
>> Please, follow the file pattern: as_u64 -> as_uint64
>>
>>> + struct {
>>> + __u64 enable_vtl_protection : 1;
>>
>> Ditto: __u64 -> u64
>>
>>> + __u64 default_vtl_protection_mask : 4;
>>> + __u64 zero_memory_on_reset : 1;
>>> + __u64 deny_lower_vtl_startup : 1;
>>> + __u64 intercept_acceptance : 1;
>>> + __u64 intercept_enable_vtl_protection : 1;
>>> + __u64 intercept_vp_startup : 1;
>>> + __u64 intercept_cpuid_unimplemented : 1;
>>> + __u64 intercept_unrecoverable_exception : 1;
>>> + __u64 intercept_page : 1;
>>> + __u64 mbz : 51;
>>> + };
>>> +};
>>> +
>>> /*
>>> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
>>> index b4067ada02cf..9b890126e8e8 100644
>>> --- a/include/hyperv/hvhdk.h
>>> +++ b/include/hyperv/hvhdk.h
>>> @@ -479,6 +479,7 @@ struct hv_connection_info {
>>> #define HV_EVENT_FLAGS_COUNT (256 * 8)
>>> #define HV_EVENT_FLAGS_BYTE_COUNT (256)
>>> #define HV_EVENT_FLAGS32_COUNT (256 / sizeof(u32))
>>> +#define HV_EVENT_FLAGS_LONG_COUNT (HV_EVENT_FLAGS_BYTE_COUNT /
>>> sizeof(__u64))
>>
>> Ditto
>>
>>> /* linux side we create long version of flags to use long bit ops
>>> on flags */
>>> #define HV_EVENT_FLAGS_UL_COUNT (256 / sizeof(ulong))
>>> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
>>> index 876bfe4e4227..a8c39b08b39a 100644
>>> --- a/include/uapi/linux/mshv.h
>>> +++ b/include/uapi/linux/mshv.h
>>> @@ -288,4 +288,86 @@ struct mshv_get_set_vp_state {
>>> * #define MSHV_ROOT_HVCALL _IOWR(MSHV_IOCTL, 0x07,
>>> struct mshv_root_hvcall)
>>> */
>>> +/* Structure definitions, macros and IOCTLs for mshv_vtl */
>>> +
>>> +#define MSHV_CAP_CORE_API_STABLE 0x0
>>> +#define MSHV_CAP_REGISTER_PAGE 0x1
>>> +#define MSHV_CAP_VTL_RETURN_ACTION 0x2
>>> +#define MSHV_CAP_DR6_SHARED 0x3
>>> +#define MSHV_MAX_RUN_MSG_SIZE 256
>>> +
>>> +#define MSHV_VP_MAX_REGISTERS 128
>>> +
>>> +struct mshv_vp_registers {
>>> + __u32 count; /* at most MSHV_VP_MAX_REGISTERS */
>>
>> Same here: __u{32,64} -> u{32,64}.
>>
>> Please, address everywhere.
>>
>
> I'll take care of all of these in my next patch.
>
One concern about this change in include/uapi/linux/mshv.h.
I see the convention of using '__' family of data types in this file.
Whatever we do here, will be applicable to existing code as well.
Do you suggest we should change it to u{32,64} variants or keep
it in current form?
Regards,
Naman
>> <snip>
>>
>>> +
>>> +/* vtl device */
>>> +#define MSHV_CREATE_VTL _IOR(MSHV_IOCTL, 0x1D, char)
>>> +#define MSHV_VTL_ADD_VTL0_MEMORY _IOW(MSHV_IOCTL, 0x21, struct
>>> mshv_vtl_ram_disposition)
>>> +#define MSHV_VTL_SET_POLL_FILE _IOW(MSHV_IOCTL, 0x25, struct
>>> mshv_vtl_set_poll_file)
>>> +#define MSHV_VTL_RETURN_TO_LOWER_VTL _IO(MSHV_IOCTL, 0x27)
>>> +#define MSHV_GET_VP_REGISTERS _IOWR(MSHV_IOCTL, 0x05, struct
>>> mshv_vp_registers)
>>> +#define MSHV_SET_VP_REGISTERS _IOW(MSHV_IOCTL, 0x06, struct
>>> mshv_vp_registers)
>>> +
>>> +/* VMBus device IOCTLs */
>>> +#define MSHV_SINT_SIGNAL_EVENT _IOW(MSHV_IOCTL, 0x22, struct
>>> mshv_vtl_signal_event)
>>> +#define MSHV_SINT_POST_MESSAGE _IOW(MSHV_IOCTL, 0x23, struct
>>> mshv_vtl_sint_post_msg)
>>> +#define MSHV_SINT_SET_EVENTFD _IOW(MSHV_IOCTL, 0x24, struct
>>> mshv_vtl_set_eventfd)
>>> +#define MSHV_SINT_PAUSE_MESSAGE_STREAM _IOW(MSHV_IOCTL, 0x25,
>>> struct mshv_sint_mask)
>>> +
>>> +/* hv_hvcall device */
>>> +#define MSHV_HVCALL_SETUP _IOW(MSHV_IOCTL, 0x1E, struct
>>> mshv_vtl_hvcall_setup)
>>> +#define MSHV_HVCALL _IOWR(MSHV_IOCTL, 0x1F, struct
>>> mshv_vtl_hvcall)
>>
>> How many of these ioctls are actually used by the mshv root driver?
>> Should those which are VTl-specific be named as such (like
>> MSHV_VTL_SET_POLL_FILE)?
>> Another option would be to keep all the names generic.
>>
>> Thanks,
>> Stanislav
>
> None of the IOCTLs in mshv_vtl section, introduced in this patch is used
> by mshv_root driver. Since IOCTLs of mshv_root does not have MSHV_ROOT
> prefix, I am OK with removing MSHV_VTL_* prefix from these IOCTL names.
> You can let me know if you want me to prefix them with MSHV_VTL.
>
> Thanks again for reviewing.
>
> Regards,
> Naman
>
>>
>>> #endif
>>> --
>>> 2.34.1
>>>
>
^ permalink raw reply
* Re: [PATCH net] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Erni Sri Satya Vennela @ 2025-05-21 8:41 UTC (permalink / raw)
To: Saurabh Sengar
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf, ssengar, stable
In-Reply-To: <1747540070-11086-1-git-send-email-ssengar@linux.microsoft.com>
On Sat, May 17, 2025 at 08:47:50PM -0700, Saurabh Sengar wrote:
> The MANA driver's probe registers netdevice via the following call chain:
>
> mana_probe()
> register_netdev()
> register_netdevice()
>
> register_netdevice() calls notifier callback for netvsc driver,
> holding the netdev mutex via netdev_lock_ops().
>
> Further this netvsc notifier callback end up attempting to acquire the
> same lock again in dev_xdp_propagate() leading to deadlock.
>
> netvsc_netdev_event()
> netvsc_vf_setxdp()
> dev_xdp_propagate()
>
> This deadlock was not observed so far because net_shaper_ops was never
> set and this lock in noop in this case. Fix this by using
> netif_xdp_propagate instead of dev_xdp_propagate to avoid recursive
> locking in this path.
>
> This issue has not observed so far because net_shaper_ops was unset,
> making the lock path effectively a no-op. To prevent recursive locking
> and avoid this deadlock, replace dev_xdp_propagate() with
> netif_xdp_propagate(), which does not acquire the lock again.
>
> Also, clean up the unregistration path by removing unnecessary call to
> netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
> performs this cleanup via dev_xdp_uninstall.
>
> Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf")
> Cc: stable@vger.kernel.org
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
Built and booted successfully.
Tested-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Thanks!
^ permalink raw reply
* Re: [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-05-21 6:03 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Roman Kisel, Anirudh Rayabharam, Saurabh Sengar, Nuno Das Neves,
ALOK TIWARI, linux-kernel, linux-hyperv
In-Reply-To: <aCzQMuwQZ1Lkk7eH@skinsburskii.>
On 5/21/2025 12:25 AM, Stanislav Kinsburskii wrote:
> On Mon, May 19, 2025 at 10:26:42AM +0530, Naman Jain wrote:
>> Provide an interface for Virtual Machine Monitor like OpenVMM and its
>> use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
>> Expose devices and support IOCTLs for features like VTL creation,
>> VTL0 memory management, context switch, making hypercalls,
>> mapping VTL0 address space to VTL2 userspace, getting new VMBus
>> messages and channel events in VTL2 etc.
>>
>> Co-developed-by: Roman Kisel <romank@linux.microsoft.com>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>> Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>> Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
>> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
>> Message-ID: <20250512140432.2387503-3-namjain@linux.microsoft.com>
>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>> ---
>> drivers/hv/Kconfig | 20 +
>> drivers/hv/Makefile | 7 +-
>> drivers/hv/mshv_vtl.h | 52 +
>> drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
>> include/hyperv/hvgdk_mini.h | 81 ++
>> include/hyperv/hvhdk.h | 1 +
>> include/uapi/linux/mshv.h | 82 ++
>> 7 files changed, 2025 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/hv/mshv_vtl.h
>> create mode 100644 drivers/hv/mshv_vtl_main.c
>>
>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
>> index eefa0b559b73..21cee5564d70 100644
>> --- a/drivers/hv/Kconfig
>> +++ b/drivers/hv/Kconfig
>> @@ -72,4 +72,24 @@ config MSHV_ROOT
>>
>> If unsure, say N.
>>
>> +config MSHV_VTL
>> + tristate "Microsoft Hyper-V VTL driver"
>> + depends on HYPERV && X86_64
>> + depends on TRANSPARENT_HUGEPAGE
>
> Why does it depend on TRANSPARENT_HUGEPAGE?
>
Thanks for reviewing. This config is required for below functions which
are used for mshv_vtl_low device.
vm_fault_t mshv_vtl_low_huge_fault ->
* vmf_insert_pfn_pmd
* vmf_insert_pfn_pud
> <snip>
>
>> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
>> index 1be7f6a02304..cc11000e39f4 100644
>> --- a/include/hyperv/hvgdk_mini.h
>> +++ b/include/hyperv/hvgdk_mini.h
>> @@ -882,6 +882,23 @@ struct hv_get_vp_from_apic_id_in {
>> u32 apic_ids[];
>> } __packed;
>>
>> +union hv_register_vsm_partition_config {
>> + __u64 as_u64;
>
> Please, follow the file pattern: as_u64 -> as_uint64
>
>> + struct {
>> + __u64 enable_vtl_protection : 1;
>
> Ditto: __u64 -> u64
>
>> + __u64 default_vtl_protection_mask : 4;
>> + __u64 zero_memory_on_reset : 1;
>> + __u64 deny_lower_vtl_startup : 1;
>> + __u64 intercept_acceptance : 1;
>> + __u64 intercept_enable_vtl_protection : 1;
>> + __u64 intercept_vp_startup : 1;
>> + __u64 intercept_cpuid_unimplemented : 1;
>> + __u64 intercept_unrecoverable_exception : 1;
>> + __u64 intercept_page : 1;
>> + __u64 mbz : 51;
>> + };
>> +};
>> +
>
>> /*
>> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
>> index b4067ada02cf..9b890126e8e8 100644
>> --- a/include/hyperv/hvhdk.h
>> +++ b/include/hyperv/hvhdk.h
>> @@ -479,6 +479,7 @@ struct hv_connection_info {
>> #define HV_EVENT_FLAGS_COUNT (256 * 8)
>> #define HV_EVENT_FLAGS_BYTE_COUNT (256)
>> #define HV_EVENT_FLAGS32_COUNT (256 / sizeof(u32))
>> +#define HV_EVENT_FLAGS_LONG_COUNT (HV_EVENT_FLAGS_BYTE_COUNT / sizeof(__u64))
>
> Ditto
>
>>
>> /* linux side we create long version of flags to use long bit ops on flags */
>> #define HV_EVENT_FLAGS_UL_COUNT (256 / sizeof(ulong))
>> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
>> index 876bfe4e4227..a8c39b08b39a 100644
>> --- a/include/uapi/linux/mshv.h
>> +++ b/include/uapi/linux/mshv.h
>> @@ -288,4 +288,86 @@ struct mshv_get_set_vp_state {
>> * #define MSHV_ROOT_HVCALL _IOWR(MSHV_IOCTL, 0x07, struct mshv_root_hvcall)
>> */
>>
>> +/* Structure definitions, macros and IOCTLs for mshv_vtl */
>> +
>> +#define MSHV_CAP_CORE_API_STABLE 0x0
>> +#define MSHV_CAP_REGISTER_PAGE 0x1
>> +#define MSHV_CAP_VTL_RETURN_ACTION 0x2
>> +#define MSHV_CAP_DR6_SHARED 0x3
>> +#define MSHV_MAX_RUN_MSG_SIZE 256
>> +
>> +#define MSHV_VP_MAX_REGISTERS 128
>> +
>> +struct mshv_vp_registers {
>> + __u32 count; /* at most MSHV_VP_MAX_REGISTERS */
>
> Same here: __u{32,64} -> u{32,64}.
>
> Please, address everywhere.
>
I'll take care of all of these in my next patch.
> <snip>
>
>> +
>> +/* vtl device */
>> +#define MSHV_CREATE_VTL _IOR(MSHV_IOCTL, 0x1D, char)
>> +#define MSHV_VTL_ADD_VTL0_MEMORY _IOW(MSHV_IOCTL, 0x21, struct mshv_vtl_ram_disposition)
>> +#define MSHV_VTL_SET_POLL_FILE _IOW(MSHV_IOCTL, 0x25, struct mshv_vtl_set_poll_file)
>> +#define MSHV_VTL_RETURN_TO_LOWER_VTL _IO(MSHV_IOCTL, 0x27)
>> +#define MSHV_GET_VP_REGISTERS _IOWR(MSHV_IOCTL, 0x05, struct mshv_vp_registers)
>> +#define MSHV_SET_VP_REGISTERS _IOW(MSHV_IOCTL, 0x06, struct mshv_vp_registers)
>> +
>> +/* VMBus device IOCTLs */
>> +#define MSHV_SINT_SIGNAL_EVENT _IOW(MSHV_IOCTL, 0x22, struct mshv_vtl_signal_event)
>> +#define MSHV_SINT_POST_MESSAGE _IOW(MSHV_IOCTL, 0x23, struct mshv_vtl_sint_post_msg)
>> +#define MSHV_SINT_SET_EVENTFD _IOW(MSHV_IOCTL, 0x24, struct mshv_vtl_set_eventfd)
>> +#define MSHV_SINT_PAUSE_MESSAGE_STREAM _IOW(MSHV_IOCTL, 0x25, struct mshv_sint_mask)
>> +
>> +/* hv_hvcall device */
>> +#define MSHV_HVCALL_SETUP _IOW(MSHV_IOCTL, 0x1E, struct mshv_vtl_hvcall_setup)
>> +#define MSHV_HVCALL _IOWR(MSHV_IOCTL, 0x1F, struct mshv_vtl_hvcall)
>
> How many of these ioctls are actually used by the mshv root driver?
> Should those which are VTl-specific be named as such (like MSHV_VTL_SET_POLL_FILE)?
> Another option would be to keep all the names generic.
>
> Thanks,
> Stanislav
None of the IOCTLs in mshv_vtl section, introduced in this patch is used
by mshv_root driver. Since IOCTLs of mshv_root does not have MSHV_ROOT
prefix, I am OK with removing MSHV_VTL_* prefix from these IOCTL names.
You can let me know if you want me to prefix them with MSHV_VTL.
Thanks again for reviewing.
Regards,
Naman
>
>> #endif
>> --
>> 2.34.1
>>
^ permalink raw reply
* Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
From: Kees Cook @ 2025-05-21 5:18 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Kuniyuki Iwashima, ahmed.zaki, aleksander.lobakin, alex.aring,
andrew+netdev, ardb, christophe.leroy, cratiu, d.bogdanov, davem,
decui, dianders, ebiggers, edumazet, fercerpav, gmazyland,
grundler, haiyangz, hayeswang, hch, horms, idosch, jiri, jv, kch,
kys, leiyang, linux-hardening, linux-hyperv, linux-kernel,
linux-nvme, linux-scsi, linux-usb, linux-wpan, linux,
martin.petersen, mgurtovoy, michael.christie, mingzhe.zou,
miquel.raynal, mlombard, netdev, pabeni, phahn-oss, sagi, sam,
sdf, shaw.leon, stefan, target-devel, viro, wei.liu
In-Reply-To: <20250520200929.1b9ae5ec@kernel.org>
On May 20, 2025 8:09:29 PM PDT, Jakub Kicinski <kuba@kernel.org> wrote:
>On Tue, 20 May 2025 17:42:32 -0700 Kees Cook wrote:
>> Ah yes, I can include that in the next version if you want? I was trying
>> to find a stopping point since everything kind of touches everything ...
>
>Looks like the build considers -Wincompatible-pointer-types to always
>imply -Werror or some such? We explicitly disable CONFIG_WERROR in our
>CI, but we still get:
>
>drivers/net/macvlan.c:1302:34: error: incompatible pointer types passing 'struct sockaddr *' to parameter of type 'struct __kernel_sockaddr_storage *' [-Werror,-Wincompatible-pointer-types]
> 1302 | dev_set_mac_address(port->dev, &sa, NULL);
> | ^~~
>
>on this series :(
I'll get this fixed and add dev_set_mac_address_user() for v3...
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
From: Jakub Kicinski @ 2025-05-21 3:09 UTC (permalink / raw)
To: Kees Cook
Cc: Kuniyuki Iwashima, ahmed.zaki, aleksander.lobakin, alex.aring,
andrew+netdev, ardb, christophe.leroy, cratiu, d.bogdanov, davem,
decui, dianders, ebiggers, edumazet, fercerpav, gmazyland,
grundler, haiyangz, hayeswang, hch, horms, idosch, jiri, jv, kch,
kys, leiyang, linux-hardening, linux-hyperv, linux-kernel,
linux-nvme, linux-scsi, linux-usb, linux-wpan, linux,
martin.petersen, mgurtovoy, michael.christie, mingzhe.zou,
miquel.raynal, mlombard, netdev, pabeni, phahn-oss, sagi, sam,
sdf, shaw.leon, stefan, target-devel, viro, wei.liu
In-Reply-To: <202505201741.AFA146E7F6@keescook>
On Tue, 20 May 2025 17:42:32 -0700 Kees Cook wrote:
> Ah yes, I can include that in the next version if you want? I was trying
> to find a stopping point since everything kind of touches everything ...
Looks like the build considers -Wincompatible-pointer-types to always
imply -Werror or some such? We explicitly disable CONFIG_WERROR in our
CI, but we still get:
drivers/net/macvlan.c:1302:34: error: incompatible pointer types passing 'struct sockaddr *' to parameter of type 'struct __kernel_sockaddr_storage *' [-Werror,-Wincompatible-pointer-types]
1302 | dev_set_mac_address(port->dev, &sa, NULL);
| ^~~
on this series :(
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH 6/7] net: core: Convert dev_set_mac_address() to struct sockaddr_storage
From: Gustavo A. R. Silva @ 2025-05-20 22:50 UTC (permalink / raw)
To: Kees Cook, Kuniyuki Iwashima
Cc: Jakub Kicinski, Jay Vosburgh, Andrew Lunn, David S. Miller,
Eric Dumazet, Paolo Abeni, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Simon Horman, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Samuel Mendoza-Jonas, Paul Fertser,
Hayes Wang, Douglas Anderson, Grant Grundler, Stanislav Fomichev,
Cosmin Ratiu, Lei Yang, netdev, linux-hyperv, linux-usb,
linux-wpan, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
Martin K. Petersen, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Ido Schimmel, Eric Biggers, Milan Broz,
Philipp Hahn, Ard Biesheuvel, Al Viro, Ahmed Zaki,
Alexander Lobakin, Xiao Liang, linux-kernel, linux-nvme,
linux-scsi, target-devel, linux-hardening
In-Reply-To: <20250520223108.2672023-6-kees@kernel.org>
On 20/05/25 16:31, Kees Cook wrote:
> All users of dev_set_mac_address() are now using a struct sockaddr_storage.
> Convert the internal data type to struct sockaddr_storage, drop the casts,
> and update pointer types.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks!
-Gustavo
> ---
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Jay Vosburgh <jv@jvosburgh.net>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Wei Liu <wei.liu@kernel.org>
> Cc: Dexuan Cui <decui@microsoft.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Alexander Aring <alex.aring@gmail.com>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> Cc: Paul Fertser <fercerpav@gmail.com>
> Cc: Hayes Wang <hayeswang@realtek.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Grant Grundler <grundler@chromium.org>
> Cc: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Cosmin Ratiu <cratiu@nvidia.com>
> Cc: Lei Yang <leiyang@redhat.com>
> Cc: <netdev@vger.kernel.org>
> Cc: <linux-hyperv@vger.kernel.org>
> Cc: <linux-usb@vger.kernel.org>
> Cc: <linux-wpan@vger.kernel.org>
> ---
> include/linux/netdevice.h | 2 +-
> drivers/net/bonding/bond_alb.c | 8 +++-----
> drivers/net/bonding/bond_main.c | 10 ++++------
> drivers/net/hyperv/netvsc_drv.c | 6 +++---
> drivers/net/macvlan.c | 10 +++++-----
> drivers/net/team/team_core.c | 2 +-
> drivers/net/usb/r8152.c | 2 +-
> net/core/dev.c | 1 +
> net/core/dev_api.c | 6 +++---
> net/ieee802154/nl-phy.c | 2 +-
> net/ncsi/ncsi-manage.c | 2 +-
> 11 files changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 47200a394a02..b4242b997373 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4214,7 +4214,7 @@ int dev_pre_changeaddr_notify(struct net_device *dev, const char *addr,
> struct netlink_ext_ack *extack);
> int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
> struct netlink_ext_ack *extack);
> -int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
> +int dev_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
> struct netlink_ext_ack *extack);
> int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
> struct netlink_ext_ack *extack);
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 7edf0fd58c34..2d37b07c8215 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -1035,7 +1035,7 @@ static int alb_set_slave_mac_addr(struct slave *slave, const u8 addr[],
> */
> memcpy(ss.__data, addr, len);
> ss.ss_family = dev->type;
> - if (dev_set_mac_address(dev, (struct sockaddr *)&ss, NULL)) {
> + if (dev_set_mac_address(dev, &ss, NULL)) {
> slave_err(slave->bond->dev, dev, "dev_set_mac_address on slave failed! ALB mode requires that the base driver support setting the hw address also when the network device's interface is open\n");
> return -EOPNOTSUPP;
> }
> @@ -1273,8 +1273,7 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
> break;
> bond_hw_addr_copy(tmp_addr, rollback_slave->dev->dev_addr,
> rollback_slave->dev->addr_len);
> - dev_set_mac_address(rollback_slave->dev,
> - (struct sockaddr *)&ss, NULL);
> + dev_set_mac_address(rollback_slave->dev, &ss, NULL);
> dev_addr_set(rollback_slave->dev, tmp_addr);
> }
>
> @@ -1763,8 +1762,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
> bond->dev->addr_len);
> ss.ss_family = bond->dev->type;
> /* we don't care if it can't change its mac, best effort */
> - dev_set_mac_address(new_slave->dev, (struct sockaddr *)&ss,
> - NULL);
> + dev_set_mac_address(new_slave->dev, &ss, NULL);
>
> dev_addr_set(new_slave->dev, tmp_addr);
> }
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 98cf4486fcee..b92e8935d686 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1112,8 +1112,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
> ss.ss_family = bond->dev->type;
> }
>
> - rv = dev_set_mac_address(new_active->dev,
> - (struct sockaddr *)&ss, NULL);
> + rv = dev_set_mac_address(new_active->dev, &ss, NULL);
> if (rv) {
> slave_err(bond->dev, new_active->dev, "Error %d setting MAC of new active slave\n",
> -rv);
> @@ -1127,8 +1126,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
> new_active->dev->addr_len);
> ss.ss_family = old_active->dev->type;
>
> - rv = dev_set_mac_address(old_active->dev,
> - (struct sockaddr *)&ss, NULL);
> + rv = dev_set_mac_address(old_active->dev, &ss, NULL);
> if (rv)
> slave_err(bond->dev, old_active->dev, "Error %d setting MAC of old active slave\n",
> -rv);
> @@ -2455,7 +2453,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> bond_hw_addr_copy(ss.__data, new_slave->perm_hwaddr,
> new_slave->dev->addr_len);
> ss.ss_family = slave_dev->type;
> - dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, NULL);
> + dev_set_mac_address(slave_dev, &ss, NULL);
> }
>
> err_restore_mtu:
> @@ -2649,7 +2647,7 @@ static int __bond_release_one(struct net_device *bond_dev,
> bond_hw_addr_copy(ss.__data, slave->perm_hwaddr,
> slave->dev->addr_len);
> ss.ss_family = slave_dev->type;
> - dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, NULL);
> + dev_set_mac_address(slave_dev, &ss, NULL);
> }
>
> if (unregister) {
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index d8b169ac0343..14a0d04e21ae 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1371,7 +1371,7 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
> struct net_device_context *ndc = netdev_priv(ndev);
> struct net_device *vf_netdev = rtnl_dereference(ndc->vf_netdev);
> struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
> - struct sockaddr *addr = p;
> + struct sockaddr_storage *addr = p;
> int err;
>
> err = eth_prepare_mac_addr_change(ndev, p);
> @@ -1387,12 +1387,12 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
> return err;
> }
>
> - err = rndis_filter_set_device_mac(nvdev, addr->sa_data);
> + err = rndis_filter_set_device_mac(nvdev, addr->__data);
> if (!err) {
> eth_commit_mac_addr_change(ndev, p);
> } else if (vf_netdev) {
> /* rollback change on VF */
> - memcpy(addr->sa_data, ndev->dev_addr, ETH_ALEN);
> + memcpy(addr->__data, ndev->dev_addr, ETH_ALEN);
> dev_set_mac_address(vf_netdev, addr, NULL);
> }
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 7045b1d58754..69e879780c36 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -754,13 +754,13 @@ static int macvlan_sync_address(struct net_device *dev,
> static int macvlan_set_mac_address(struct net_device *dev, void *p)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> - struct sockaddr *addr = p;
> + struct sockaddr_storage *addr = p;
>
> - if (!is_valid_ether_addr(addr->sa_data))
> + if (!is_valid_ether_addr(addr->__data))
> return -EADDRNOTAVAIL;
>
> /* If the addresses are the same, this is a no-op */
> - if (ether_addr_equal(dev->dev_addr, addr->sa_data))
> + if (ether_addr_equal(dev->dev_addr, addr->__data))
> return 0;
>
> if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
> @@ -768,10 +768,10 @@ static int macvlan_set_mac_address(struct net_device *dev, void *p)
> return dev_set_mac_address(vlan->lowerdev, addr, NULL);
> }
>
> - if (macvlan_addr_busy(vlan->port, addr->sa_data))
> + if (macvlan_addr_busy(vlan->port, addr->__data))
> return -EADDRINUSE;
>
> - return macvlan_sync_address(dev, addr->sa_data);
> + return macvlan_sync_address(dev, addr->__data);
> }
>
> static void macvlan_change_rx_flags(struct net_device *dev, int change)
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index d8fc0c79745d..a64e661c21a1 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -55,7 +55,7 @@ static int __set_port_dev_addr(struct net_device *port_dev,
>
> memcpy(addr.__data, dev_addr, port_dev->addr_len);
> addr.ss_family = port_dev->type;
> - return dev_set_mac_address(port_dev, (struct sockaddr *)&addr, NULL);
> + return dev_set_mac_address(port_dev, &addr, NULL);
> }
>
> static int team_port_set_orig_dev_addr(struct team_port *port)
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index b18dee1b1bb3..d6589b24c68d 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -8432,7 +8432,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
>
> /* reset the MAC address in case of policy change */
> if (determine_ethernet_addr(tp, &ss) >= 0)
> - dev_set_mac_address(tp->netdev, (struct sockaddr *)&ss, NULL);
> + dev_set_mac_address(tp->netdev, &ss, NULL);
>
> netdev = tp->netdev;
> if (!netif_running(netdev))
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f8c8aad7df2e..1f1900ec26b2 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9683,6 +9683,7 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
>
> DECLARE_RWSEM(dev_addr_sem);
>
> +/* "sa" is a true struct sockaddr with limited "sa_data" member. */
> int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name)
> {
> size_t size = sizeof(sa->sa_data_min);
> diff --git a/net/core/dev_api.c b/net/core/dev_api.c
> index b5f293e637d9..e80404e76ca9 100644
> --- a/net/core/dev_api.c
> +++ b/net/core/dev_api.c
> @@ -319,20 +319,20 @@ EXPORT_SYMBOL(dev_set_allmulti);
> /**
> * dev_set_mac_address() - change Media Access Control Address
> * @dev: device
> - * @sa: new address
> + * @ss: new address
> * @extack: netlink extended ack
> *
> * Change the hardware (MAC) address of the device
> *
> * Return: 0 on success, -errno on failure.
> */
> -int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
> +int dev_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
> struct netlink_ext_ack *extack)
> {
> int ret;
>
> netdev_lock_ops(dev);
> - ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
> + ret = netif_set_mac_address(dev, sa, extack);
> netdev_unlock_ops(dev);
>
> return ret;
> diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
> index ee2b190e8e0d..4c07a475c567 100644
> --- a/net/ieee802154/nl-phy.c
> +++ b/net/ieee802154/nl-phy.c
> @@ -234,7 +234,7 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
> * dev_set_mac_address require RTNL_LOCK
> */
> rtnl_lock();
> - rc = dev_set_mac_address(dev, (struct sockaddr *)&addr, NULL);
> + rc = dev_set_mac_address(dev, &addr, NULL);
> rtnl_unlock();
> if (rc)
> goto dev_unregister;
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 0202db2aea3e..b36947063783 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1058,7 +1058,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> break;
> case ncsi_dev_state_config_apply_mac:
> rtnl_lock();
> - ret = dev_set_mac_address(dev, (struct sockaddr *)&ndp->pending_mac, NULL);
> + ret = dev_set_mac_address(dev, &ndp->pending_mac, NULL);
> rtnl_unlock();
> if (ret < 0)
> netdev_warn(dev, "NCSI: 'Writing MAC address to device failed\n");
^ permalink raw reply
* Re: [PATCH 4/7] ieee802154: Use struct sockaddr_storage with dev_set_mac_address()
From: Gustavo A. R. Silva @ 2025-05-20 22:49 UTC (permalink / raw)
To: Kees Cook, Kuniyuki Iwashima
Cc: Alexander Aring, Stefan Schmidt, Miquel Raynal, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
linux-wpan, netdev, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni, Martin K. Petersen, Mike Christie,
Max Gurtovoy, Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou,
Christophe Leroy, Dr. David Alan Gilbert, Andrew Lunn,
Stanislav Fomichev, Cosmin Ratiu, Lei Yang, Ido Schimmel,
Samuel Mendoza-Jonas, Paul Fertser, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Eric Biggers, Milan Broz,
Philipp Hahn, Ard Biesheuvel, Al Viro, Ahmed Zaki,
Alexander Lobakin, Xiao Liang, linux-kernel, linux-nvme,
linux-scsi, target-devel, linux-usb, linux-hyperv,
linux-hardening
In-Reply-To: <20250520223108.2672023-4-kees@kernel.org>
On 20/05/25 16:31, Kees Cook wrote:
> Switch to struct sockaddr_storage for calling dev_set_mac_address(). Add
> a temporary cast to struct sockaddr, which will be removed in a
> subsequent patch.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks!
-Gustavo
> ---
> Cc: Alexander Aring <alex.aring@gmail.com>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: <linux-wpan@vger.kernel.org>
> Cc: <netdev@vger.kernel.org>
> ---
> net/ieee802154/nl-phy.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
> index 359249ab77bf..ee2b190e8e0d 100644
> --- a/net/ieee802154/nl-phy.c
> +++ b/net/ieee802154/nl-phy.c
> @@ -224,17 +224,17 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
> dev_hold(dev);
>
> if (info->attrs[IEEE802154_ATTR_HW_ADDR]) {
> - struct sockaddr addr;
> + struct sockaddr_storage addr;
>
> - addr.sa_family = ARPHRD_IEEE802154;
> - nla_memcpy(&addr.sa_data, info->attrs[IEEE802154_ATTR_HW_ADDR],
> + addr.ss_family = ARPHRD_IEEE802154;
> + nla_memcpy(&addr.__data, info->attrs[IEEE802154_ATTR_HW_ADDR],
> IEEE802154_ADDR_LEN);
>
> /* strangely enough, some callbacks (inetdev_event) from
> * dev_set_mac_address require RTNL_LOCK
> */
> rtnl_lock();
> - rc = dev_set_mac_address(dev, &addr, NULL);
> + rc = dev_set_mac_address(dev, (struct sockaddr *)&addr, NULL);
> rtnl_unlock();
> if (rc)
> goto dev_unregister;
^ permalink raw reply
* Re: [PATCH 3/7] net/ncsi: Use struct sockaddr_storage for pending_mac
From: Gustavo A. R. Silva @ 2025-05-20 22:48 UTC (permalink / raw)
To: Kees Cook, Kuniyuki Iwashima
Cc: Samuel Mendoza-Jonas, Paul Fertser, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, netdev,
Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
Martin K. Petersen, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Andrew Lunn, Stanislav Fomichev,
Cosmin Ratiu, Lei Yang, Ido Schimmel, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Eric Biggers, Milan Broz,
Philipp Hahn, Ard Biesheuvel, Al Viro, Ahmed Zaki,
Alexander Lobakin, Xiao Liang, linux-kernel, linux-nvme,
linux-scsi, target-devel, linux-wpan, linux-usb, linux-hyperv,
linux-hardening
In-Reply-To: <20250520223108.2672023-3-kees@kernel.org>
On 20/05/25 16:31, Kees Cook wrote:
> To avoid future casting with coming API type changes, switch struct
> ncsi_dev_priv::pending_mac to a full struct sockaddr_storage.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks!
-Gustavo
> ---
> Cc: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> Cc: Paul Fertser <fercerpav@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: <netdev@vger.kernel.org>
> ---
> net/ncsi/internal.h | 2 +-
> net/ncsi/ncsi-manage.c | 2 +-
> net/ncsi/ncsi-rsp.c | 18 +++++++++---------
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 2c260f33b55c..e76c6de0c784 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -322,7 +322,7 @@ struct ncsi_dev_priv {
> #define NCSI_DEV_RESHUFFLE 4
> #define NCSI_DEV_RESET 8 /* Reset state of NC */
> unsigned int gma_flag; /* OEM GMA flag */
> - struct sockaddr pending_mac; /* MAC address received from GMA */
> + struct sockaddr_storage pending_mac; /* MAC address received from GMA */
> spinlock_t lock; /* Protect the NCSI device */
> unsigned int package_probe_id;/* Current ID during probe */
> unsigned int package_num; /* Number of packages */
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index b36947063783..0202db2aea3e 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1058,7 +1058,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> break;
> case ncsi_dev_state_config_apply_mac:
> rtnl_lock();
> - ret = dev_set_mac_address(dev, &ndp->pending_mac, NULL);
> + ret = dev_set_mac_address(dev, (struct sockaddr *)&ndp->pending_mac, NULL);
> rtnl_unlock();
> if (ret < 0)
> netdev_warn(dev, "NCSI: 'Writing MAC address to device failed\n");
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index 8668888c5a2f..472cc68ad86f 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -628,7 +628,7 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
> static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
> {
> struct ncsi_dev_priv *ndp = nr->ndp;
> - struct sockaddr *saddr = &ndp->pending_mac;
> + struct sockaddr_storage *saddr = &ndp->pending_mac;
> struct net_device *ndev = ndp->ndev.dev;
> struct ncsi_rsp_oem_pkt *rsp;
> u32 mac_addr_off = 0;
> @@ -644,11 +644,11 @@ static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
> else if (mfr_id == NCSI_OEM_MFR_INTEL_ID)
> mac_addr_off = INTEL_MAC_ADDR_OFFSET;
>
> - saddr->sa_family = ndev->type;
> - memcpy(saddr->sa_data, &rsp->data[mac_addr_off], ETH_ALEN);
> + saddr->ss_family = ndev->type;
> + memcpy(saddr->__data, &rsp->data[mac_addr_off], ETH_ALEN);
> if (mfr_id == NCSI_OEM_MFR_BCM_ID || mfr_id == NCSI_OEM_MFR_INTEL_ID)
> - eth_addr_inc((u8 *)saddr->sa_data);
> - if (!is_valid_ether_addr((const u8 *)saddr->sa_data))
> + eth_addr_inc(saddr->__data);
> + if (!is_valid_ether_addr(saddr->__data))
> return -ENXIO;
>
> /* Set the flag for GMA command which should only be called once */
> @@ -1088,7 +1088,7 @@ static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
> static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
> {
> struct ncsi_dev_priv *ndp = nr->ndp;
> - struct sockaddr *saddr = &ndp->pending_mac;
> + struct sockaddr_storage *saddr = &ndp->pending_mac;
> struct net_device *ndev = ndp->ndev.dev;
> struct ncsi_rsp_gmcma_pkt *rsp;
> int i;
> @@ -1105,15 +1105,15 @@ static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
> rsp->addresses[i][4], rsp->addresses[i][5]);
> }
>
> - saddr->sa_family = ndev->type;
> + saddr->ss_family = ndev->type;
> for (i = 0; i < rsp->address_count; i++) {
> if (!is_valid_ether_addr(rsp->addresses[i])) {
> netdev_warn(ndev, "NCSI: Unable to assign %pM to device\n",
> rsp->addresses[i]);
> continue;
> }
> - memcpy(saddr->sa_data, rsp->addresses[i], ETH_ALEN);
> - netdev_warn(ndev, "NCSI: Will set MAC address to %pM\n", saddr->sa_data);
> + memcpy(saddr->__data, rsp->addresses[i], ETH_ALEN);
> + netdev_warn(ndev, "NCSI: Will set MAC address to %pM\n", saddr->__data);
> break;
> }
>
^ permalink raw reply
* Re: [PATCH 1/7] net: core: Convert inet_addr_is_any() to sockaddr_storage
From: Martin K. Petersen @ 2025-05-21 1:26 UTC (permalink / raw)
To: Kees Cook
Cc: Kuniyuki Iwashima, Gustavo A . R . Silva, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Martin K. Petersen,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Mike Christie, Max Gurtovoy, Maurizio Lombardi, Dmitry Bogdanov,
Mingzhe Zou, Christophe Leroy, Simon Horman,
Dr. David Alan Gilbert, linux-nvme, linux-scsi, target-devel,
netdev, Andrew Lunn, Stanislav Fomichev, Cosmin Ratiu, Lei Yang,
Ido Schimmel, Samuel Mendoza-Jonas, Paul Fertser, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Eric Biggers, Milan Broz,
Philipp Hahn, Ard Biesheuvel, Al Viro, Ahmed Zaki,
Alexander Lobakin, Xiao Liang, linux-kernel, linux-wpan,
linux-usb, linux-hyperv, linux-hardening
In-Reply-To: <20250520223108.2672023-1-kees@kernel.org>
Kees,
> All the callers of inet_addr_is_any() have a sockaddr_storage-backed
> sockaddr. Avoid casts and switch prototype to the actual object being
> used.
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> # SCSI
--
Martin K. Petersen
^ permalink raw reply
* Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
From: Kees Cook @ 2025-05-21 0:42 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: ahmed.zaki, aleksander.lobakin, alex.aring, andrew+netdev, ardb,
christophe.leroy, cratiu, d.bogdanov, davem, decui, dianders,
ebiggers, edumazet, fercerpav, gmazyland, grundler, haiyangz,
hayeswang, hch, horms, idosch, jiri, jv, kch, kuba, kys, leiyang,
linux-hardening, linux-hyperv, linux-kernel, linux-nvme,
linux-scsi, linux-usb, linux-wpan, linux, martin.petersen,
mgurtovoy, michael.christie, mingzhe.zou, miquel.raynal, mlombard,
netdev, pabeni, phahn-oss, sagi, sam, sdf, shaw.leon, stefan,
target-devel, viro, wei.liu
In-Reply-To: <20250521001931.7761-1-kuniyu@amazon.com>
On Tue, May 20, 2025 at 05:19:20PM -0700, Kuniyuki Iwashima wrote:
> From: Kees Cook <kees@kernel.org>
> Date: Tue, 20 May 2025 15:30:59 -0700
> > Hi,
> >
> > As part of the effort to allow the compiler to reason about object sizes,
> > we need to deal with the problematic variably sized struct sockaddr,
> > which has no internal runtime size tracking. In much of the network
> > stack the use of struct sockaddr_storage has been adopted. Continue the
> > transition toward this for more of the internal APIs. Specifically:
> >
> > - inet_addr_is_any()
> > - netif_set_mac_address()
> > - dev_set_mac_address()
> >
> > Only 3 callers of dev_set_mac_address() needed adjustment; all others
> > were already using struct sockaddr_storage internally.
>
> I guess dev_set_mac_address_user() was missed on the way ?
>
> For example, tap_ioctl() still uses sockaddr and calls
> dev_set_mac_address_user(), which cast it to _storage.
Ah yes, I can include that in the next version if you want? I was trying
to find a stopping point since everything kind of touches everything ...
:P
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
From: Kuniyuki Iwashima @ 2025-05-21 0:19 UTC (permalink / raw)
To: kees
Cc: ahmed.zaki, aleksander.lobakin, alex.aring, andrew+netdev, ardb,
christophe.leroy, cratiu, d.bogdanov, davem, decui, dianders,
ebiggers, edumazet, fercerpav, gmazyland, grundler, haiyangz,
hayeswang, hch, horms, idosch, jiri, jv, kch, kuba, kuniyu, kys,
leiyang, linux-hardening, linux-hyperv, linux-kernel, linux-nvme,
linux-scsi, linux-usb, linux-wpan, linux, martin.petersen,
mgurtovoy, michael.christie, mingzhe.zou, miquel.raynal, mlombard,
netdev, pabeni, phahn-oss, sagi, sam, sdf, shaw.leon, stefan,
target-devel, viro, wei.liu
In-Reply-To: <20250520222452.work.063-kees@kernel.org>
From: Kees Cook <kees@kernel.org>
Date: Tue, 20 May 2025 15:30:59 -0700
> Hi,
>
> As part of the effort to allow the compiler to reason about object sizes,
> we need to deal with the problematic variably sized struct sockaddr,
> which has no internal runtime size tracking. In much of the network
> stack the use of struct sockaddr_storage has been adopted. Continue the
> transition toward this for more of the internal APIs. Specifically:
>
> - inet_addr_is_any()
> - netif_set_mac_address()
> - dev_set_mac_address()
>
> Only 3 callers of dev_set_mac_address() needed adjustment; all others
> were already using struct sockaddr_storage internally.
I guess dev_set_mac_address_user() was missed on the way ?
For example, tap_ioctl() still uses sockaddr and calls
dev_set_mac_address_user(), which cast it to _storage.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox