linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge
@ 2021-08-09 13:11 Vladimir Oltean
  2021-08-09 15:16 ` Ido Schimmel
  2021-08-10  6:50 ` Leon Romanovsky
  0 siblings, 2 replies; 6+ messages in thread
From: Vladimir Oltean @ 2021-08-09 13:11 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Vadym Kochan, Taras Chornyi, Saeed Mahameed, Leon Romanovsky,
	Jiri Pirko, Ido Schimmel, Lars Povlsen, Steen Hegelund,
	UNGLinuxDriver, Grygorii Strashko, Julian Wiedmann,
	Karsten Graul, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Jianbo Liu, Vlad Buslov, Bjarni Jonasson,
	Vignesh Raghavendra, Tobias Waldekranz, linux-kernel, linux-rdma,
	linux-arm-kernel, linux-omap, linux-s390, Ido Schimmel

The blamed commit a new field to struct switchdev_notifier_fdb_info, but
did not make sure that all call paths set it to something valid. For
example, a switchdev driver may emit a SWITCHDEV_FDB_ADD_TO_BRIDGE
notifier, and since the 'is_local' flag is not set, it contains junk
from the stack, so the bridge might interpret those notifications as
being for local FDB entries when that was not intended.

To avoid that now and in the future, zero-initialize all
switchdev_notifier_fdb_info structures created by drivers such that all
newly added fields to not need to touch drivers again.

Fixes: 2c4eca3ef716 ("net: bridge: switchdev: include local flag in FDB notifications")
Reported-by: Ido Schimmel <idosch@idosch.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/marvell/prestera/prestera_switchdev.c | 2 ++
 drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c       | 1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c      | 2 ++
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c   | 1 +
 drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c    | 1 +
 drivers/net/ethernet/rocker/rocker_main.c                  | 1 +
 drivers/net/ethernet/rocker/rocker_ofdpa.c                 | 1 +
 drivers/net/ethernet/ti/am65-cpsw-switchdev.c              | 1 +
 drivers/net/ethernet/ti/cpsw_switchdev.c                   | 1 +
 drivers/s390/net/qeth_l2_main.c                            | 2 ++
 net/dsa/slave.c                                            | 1 +
 11 files changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
index 0b3e8f2db294..cf60e80dd3ba 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
@@ -750,6 +750,7 @@ prestera_fdb_offload_notify(struct prestera_port *port,
 {
 	struct switchdev_notifier_fdb_info send_info;
 
+	memset(&send_info, 0, sizeof(send_info));
 	send_info.addr = info->addr;
 	send_info.vid = info->vid;
 	send_info.offloaded = true;
@@ -1146,6 +1147,7 @@ static void prestera_fdb_event(struct prestera_switch *sw,
 	if (!dev)
 		return;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = evt->fdb_evt.data.mac;
 	info.vid = evt->fdb_evt.vid;
 	info.offloaded = true;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
index a6e1d4f78268..77e09397a062 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
@@ -71,6 +71,7 @@ mlx5_esw_bridge_fdb_offload_notify(struct net_device *dev, const unsigned char *
 {
 	struct switchdev_notifier_fdb_info send_info;
 
+	memset(&send_info, 0, sizeof(send_info));
 	send_info.addr = addr;
 	send_info.vid = vid;
 	send_info.offloaded = true;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 7e221ef01437..8a7660f2d048 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -9086,6 +9086,7 @@ static void mlxsw_sp_rif_fid_fdb_del(struct mlxsw_sp_rif *rif, const char *mac)
 	if (!dev)
 		return;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = mac;
 	info.vid = 0;
 	call_switchdev_notifiers(SWITCHDEV_FDB_DEL_TO_BRIDGE, dev, &info.info,
@@ -9137,6 +9138,7 @@ static void mlxsw_sp_rif_vlan_fdb_del(struct mlxsw_sp_rif *rif, const char *mac)
 	if (!dev)
 		return;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = mac;
 	info.vid = vid;
 	call_switchdev_notifiers(SWITCHDEV_FDB_DEL_TO_BRIDGE, dev, &info.info,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index c5ef9aa64efe..f016d909bead 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -2510,6 +2510,7 @@ mlxsw_sp_fdb_call_notifiers(enum switchdev_notifier_type type,
 {
 	struct switchdev_notifier_fdb_info info;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = mac;
 	info.vid = vid;
 	info.offloaded = offloaded;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
index 0443f66b5550..fbc3f5e65882 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
@@ -279,6 +279,7 @@ static void sparx5_fdb_call_notifiers(enum switchdev_notifier_type type,
 {
 	struct switchdev_notifier_fdb_info info;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = mac;
 	info.vid = vid;
 	info.offloaded = offloaded;
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index a46633606cae..49d548be9fe4 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2717,6 +2717,7 @@ rocker_fdb_offload_notify(struct rocker_port *rocker_port,
 {
 	struct switchdev_notifier_fdb_info info;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = recv_info->addr;
 	info.vid = recv_info->vid;
 	info.offloaded = true;
diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index 967a634ee9ac..7d954fd24134 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -1824,6 +1824,7 @@ static void ofdpa_port_fdb_learn_work(struct work_struct *work)
 	bool learned = (lw->flags & OFDPA_OP_FLAG_LEARNED);
 	struct switchdev_notifier_fdb_info info;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = lw->addr;
 	info.vid = lw->vid;
 
diff --git a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
index 9c29b363e9ae..81d2b1765a66 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
@@ -360,6 +360,7 @@ static void am65_cpsw_fdb_offload_notify(struct net_device *ndev,
 {
 	struct switchdev_notifier_fdb_info info;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = rcv->addr;
 	info.vid = rcv->vid;
 	info.offloaded = true;
diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c
index f7fb6e17dadd..446bdab06bdd 100644
--- a/drivers/net/ethernet/ti/cpsw_switchdev.c
+++ b/drivers/net/ethernet/ti/cpsw_switchdev.c
@@ -370,6 +370,7 @@ static void cpsw_fdb_offload_notify(struct net_device *ndev,
 {
 	struct switchdev_notifier_fdb_info info;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = rcv->addr;
 	info.vid = rcv->vid;
 	info.offloaded = true;
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 2abf86c104d5..843dd4f4d8d7 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -283,6 +283,7 @@ static void qeth_l2_dev2br_fdb_flush(struct qeth_card *card)
 
 	QETH_CARD_TEXT(card, 2, "fdbflush");
 
+	memset(&info, 0, sizeof(info));
 	info.addr = NULL;
 	/* flush all VLANs: */
 	info.vid = 0;
@@ -693,6 +694,7 @@ static void qeth_l2_dev2br_fdb_notify(struct qeth_card *card, u8 code,
 	if (qeth_is_my_net_if_token(card, token))
 		return;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = ntfy_mac;
 	/* don't report VLAN IDs */
 	info.vid = 0;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 532085da8d8f..1cb7f7e56784 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2298,6 +2298,7 @@ dsa_fdb_offload_notify(struct dsa_switchdev_event_work *switchdev_work)
 	if (!dsa_is_user_port(ds, switchdev_work->port))
 		return;
 
+	memset(&info, 0, sizeof(info));
 	info.addr = switchdev_work->addr;
 	info.vid = switchdev_work->vid;
 	info.offloaded = true;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge
  2021-08-09 13:11 [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge Vladimir Oltean
@ 2021-08-09 15:16 ` Ido Schimmel
  2021-08-10  6:50 ` Leon Romanovsky
  1 sibling, 0 replies; 6+ messages in thread
From: Ido Schimmel @ 2021-08-09 15:16 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Jakub Kicinski, David S. Miller, Andrew Lunn,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean, Vadym Kochan,
	Taras Chornyi, Saeed Mahameed, Leon Romanovsky, Jiri Pirko,
	Ido Schimmel, Lars Povlsen, Steen Hegelund, UNGLinuxDriver,
	Grygorii Strashko, Julian Wiedmann, Karsten Graul,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger, Jianbo Liu,
	Vlad Buslov, Bjarni Jonasson, Vignesh Raghavendra,
	Tobias Waldekranz, linux-kernel, linux-rdma, linux-arm-kernel,
	linux-omap, linux-s390

On Mon, Aug 09, 2021 at 04:11:52PM +0300, Vladimir Oltean wrote:
> The blamed commit a new field to struct switchdev_notifier_fdb_info, but
> did not make sure that all call paths set it to something valid. For
> example, a switchdev driver may emit a SWITCHDEV_FDB_ADD_TO_BRIDGE
> notifier, and since the 'is_local' flag is not set, it contains junk
> from the stack, so the bridge might interpret those notifications as
> being for local FDB entries when that was not intended.
> 
> To avoid that now and in the future, zero-initialize all
> switchdev_notifier_fdb_info structures created by drivers such that all
> newly added fields to not need to touch drivers again.
> 
> Fixes: 2c4eca3ef716 ("net: bridge: switchdev: include local flag in FDB notifications")
> Reported-by: Ido Schimmel <idosch@idosch.org>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>

Thanks

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge
  2021-08-09 13:11 [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge Vladimir Oltean
  2021-08-09 15:16 ` Ido Schimmel
@ 2021-08-10  6:50 ` Leon Romanovsky
  2021-08-10  8:16   ` Vladimir Oltean
  1 sibling, 1 reply; 6+ messages in thread
From: Leon Romanovsky @ 2021-08-10  6:50 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Jakub Kicinski, David S. Miller, Andrew Lunn,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean, Vadym Kochan,
	Taras Chornyi, Saeed Mahameed, Jiri Pirko, Ido Schimmel,
	Lars Povlsen, Steen Hegelund, UNGLinuxDriver, Grygorii Strashko,
	Julian Wiedmann, Karsten Graul, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Jianbo Liu, Vlad Buslov, Bjarni Jonasson,
	Vignesh Raghavendra, Tobias Waldekranz, linux-kernel, linux-rdma,
	linux-arm-kernel, linux-omap, linux-s390, Ido Schimmel

On Mon, Aug 09, 2021 at 04:11:52PM +0300, Vladimir Oltean wrote:
> The blamed commit a new field to struct switchdev_notifier_fdb_info, but
> did not make sure that all call paths set it to something valid. For
> example, a switchdev driver may emit a SWITCHDEV_FDB_ADD_TO_BRIDGE
> notifier, and since the 'is_local' flag is not set, it contains junk
> from the stack, so the bridge might interpret those notifications as
> being for local FDB entries when that was not intended.
> 
> To avoid that now and in the future, zero-initialize all
> switchdev_notifier_fdb_info structures created by drivers such that all
> newly added fields to not need to touch drivers again.
> 
> Fixes: 2c4eca3ef716 ("net: bridge: switchdev: include local flag in FDB notifications")
> Reported-by: Ido Schimmel <idosch@idosch.org>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/ethernet/marvell/prestera/prestera_switchdev.c | 2 ++
>  drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c       | 1 +
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c      | 2 ++
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c   | 1 +
>  drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c    | 1 +
>  drivers/net/ethernet/rocker/rocker_main.c                  | 1 +
>  drivers/net/ethernet/rocker/rocker_ofdpa.c                 | 1 +
>  drivers/net/ethernet/ti/am65-cpsw-switchdev.c              | 1 +
>  drivers/net/ethernet/ti/cpsw_switchdev.c                   | 1 +
>  drivers/s390/net/qeth_l2_main.c                            | 2 ++
>  net/dsa/slave.c                                            | 1 +
>  11 files changed, 14 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
> index 0b3e8f2db294..cf60e80dd3ba 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
> @@ -750,6 +750,7 @@ prestera_fdb_offload_notify(struct prestera_port *port,
>  {
>  	struct switchdev_notifier_fdb_info send_info;
>  
> +	memset(&send_info, 0, sizeof(send_info));

This can be written simpler.
struct switchdev_notifier_fdb_info send_info = {};

In all places.

Thanks

>  	send_info.addr = info->addr;
>  	send_info.vid = info->vid;
>  	send_info.offloaded = true;
> @@ -1146,6 +1147,7 @@ static void prestera_fdb_event(struct prestera_switch *sw,
>  	if (!dev)
>  		return;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = evt->fdb_evt.data.mac;
>  	info.vid = evt->fdb_evt.vid;
>  	info.offloaded = true;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
> index a6e1d4f78268..77e09397a062 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
> @@ -71,6 +71,7 @@ mlx5_esw_bridge_fdb_offload_notify(struct net_device *dev, const unsigned char *
>  {
>  	struct switchdev_notifier_fdb_info send_info;
>  
> +	memset(&send_info, 0, sizeof(send_info));
>  	send_info.addr = addr;
>  	send_info.vid = vid;
>  	send_info.offloaded = true;
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 7e221ef01437..8a7660f2d048 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -9086,6 +9086,7 @@ static void mlxsw_sp_rif_fid_fdb_del(struct mlxsw_sp_rif *rif, const char *mac)
>  	if (!dev)
>  		return;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = mac;
>  	info.vid = 0;
>  	call_switchdev_notifiers(SWITCHDEV_FDB_DEL_TO_BRIDGE, dev, &info.info,
> @@ -9137,6 +9138,7 @@ static void mlxsw_sp_rif_vlan_fdb_del(struct mlxsw_sp_rif *rif, const char *mac)
>  	if (!dev)
>  		return;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = mac;
>  	info.vid = vid;
>  	call_switchdev_notifiers(SWITCHDEV_FDB_DEL_TO_BRIDGE, dev, &info.info,
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index c5ef9aa64efe..f016d909bead 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -2510,6 +2510,7 @@ mlxsw_sp_fdb_call_notifiers(enum switchdev_notifier_type type,
>  {
>  	struct switchdev_notifier_fdb_info info;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = mac;
>  	info.vid = vid;
>  	info.offloaded = offloaded;
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> index 0443f66b5550..fbc3f5e65882 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> @@ -279,6 +279,7 @@ static void sparx5_fdb_call_notifiers(enum switchdev_notifier_type type,
>  {
>  	struct switchdev_notifier_fdb_info info;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = mac;
>  	info.vid = vid;
>  	info.offloaded = offloaded;
> diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
> index a46633606cae..49d548be9fe4 100644
> --- a/drivers/net/ethernet/rocker/rocker_main.c
> +++ b/drivers/net/ethernet/rocker/rocker_main.c
> @@ -2717,6 +2717,7 @@ rocker_fdb_offload_notify(struct rocker_port *rocker_port,
>  {
>  	struct switchdev_notifier_fdb_info info;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = recv_info->addr;
>  	info.vid = recv_info->vid;
>  	info.offloaded = true;
> diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
> index 967a634ee9ac..7d954fd24134 100644
> --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
> +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
> @@ -1824,6 +1824,7 @@ static void ofdpa_port_fdb_learn_work(struct work_struct *work)
>  	bool learned = (lw->flags & OFDPA_OP_FLAG_LEARNED);
>  	struct switchdev_notifier_fdb_info info;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = lw->addr;
>  	info.vid = lw->vid;
>  
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
> index 9c29b363e9ae..81d2b1765a66 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
> @@ -360,6 +360,7 @@ static void am65_cpsw_fdb_offload_notify(struct net_device *ndev,
>  {
>  	struct switchdev_notifier_fdb_info info;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = rcv->addr;
>  	info.vid = rcv->vid;
>  	info.offloaded = true;
> diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c
> index f7fb6e17dadd..446bdab06bdd 100644
> --- a/drivers/net/ethernet/ti/cpsw_switchdev.c
> +++ b/drivers/net/ethernet/ti/cpsw_switchdev.c
> @@ -370,6 +370,7 @@ static void cpsw_fdb_offload_notify(struct net_device *ndev,
>  {
>  	struct switchdev_notifier_fdb_info info;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = rcv->addr;
>  	info.vid = rcv->vid;
>  	info.offloaded = true;
> diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
> index 2abf86c104d5..843dd4f4d8d7 100644
> --- a/drivers/s390/net/qeth_l2_main.c
> +++ b/drivers/s390/net/qeth_l2_main.c
> @@ -283,6 +283,7 @@ static void qeth_l2_dev2br_fdb_flush(struct qeth_card *card)
>  
>  	QETH_CARD_TEXT(card, 2, "fdbflush");
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = NULL;
>  	/* flush all VLANs: */
>  	info.vid = 0;
> @@ -693,6 +694,7 @@ static void qeth_l2_dev2br_fdb_notify(struct qeth_card *card, u8 code,
>  	if (qeth_is_my_net_if_token(card, token))
>  		return;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = ntfy_mac;
>  	/* don't report VLAN IDs */
>  	info.vid = 0;
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 532085da8d8f..1cb7f7e56784 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -2298,6 +2298,7 @@ dsa_fdb_offload_notify(struct dsa_switchdev_event_work *switchdev_work)
>  	if (!dsa_is_user_port(ds, switchdev_work->port))
>  		return;
>  
> +	memset(&info, 0, sizeof(info));
>  	info.addr = switchdev_work->addr;
>  	info.vid = switchdev_work->vid;
>  	info.offloaded = true;
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge
  2021-08-10  6:50 ` Leon Romanovsky
@ 2021-08-10  8:16   ` Vladimir Oltean
  2021-08-10  9:12     ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Oltean @ 2021-08-10  8:16 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: netdev, Jakub Kicinski, David S. Miller, Andrew Lunn,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean, Vadym Kochan,
	Taras Chornyi, Saeed Mahameed, Jiri Pirko, Ido Schimmel,
	Lars Povlsen, Steen Hegelund, UNGLinuxDriver, Grygorii Strashko,
	Julian Wiedmann, Karsten Graul, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Jianbo Liu, Vlad Buslov, Bjarni Jonasson,
	Vignesh Raghavendra, Tobias Waldekranz, linux-kernel, linux-rdma,
	linux-arm-kernel, linux-omap, linux-s390, Ido Schimmel

Hi Leon,

On Tue, Aug 10, 2021 at 09:50:41AM +0300, Leon Romanovsky wrote:
> > +	memset(&send_info, 0, sizeof(send_info));
> 
> This can be written simpler.
> struct switchdev_notifier_fdb_info send_info = {};
> 
> In all places.

Because the structure contains a sub-structure, I believe that a
compound literal initializer would require additional braces for the
initialization of its sub-objects too. At least I know that expressions
like that have attracted the attention of clang people in the past:
https://patchwork.ozlabs.org/project/netdev/patch/20190506202447.30907-1-natechancellor@gmail.com/
So I went for the 'unambiguous' path.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge
  2021-08-10  8:16   ` Vladimir Oltean
@ 2021-08-10  9:12     ` Russell King - ARM Linux admin
  2021-08-10  9:34       ` Vladimir Oltean
  0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2021-08-10  9:12 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Leon Romanovsky, netdev, Jakub Kicinski, David S. Miller,
	Andrew Lunn, Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Vadym Kochan, Taras Chornyi, Saeed Mahameed, Jiri Pirko,
	Ido Schimmel, Lars Povlsen, Steen Hegelund, UNGLinuxDriver,
	Grygorii Strashko, Julian Wiedmann, Karsten Graul,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger, Jianbo Liu,
	Vlad Buslov, Bjarni Jonasson, Vignesh Raghavendra,
	Tobias Waldekranz, linux-kernel, linux-rdma, linux-arm-kernel,
	linux-omap, linux-s390, Ido Schimmel

On Tue, Aug 10, 2021 at 08:16:17AM +0000, Vladimir Oltean wrote:
> Hi Leon,
> 
> On Tue, Aug 10, 2021 at 09:50:41AM +0300, Leon Romanovsky wrote:
> > > +	memset(&send_info, 0, sizeof(send_info));
> > 
> > This can be written simpler.
> > struct switchdev_notifier_fdb_info send_info = {};
> > 
> > In all places.
> 
> Because the structure contains a sub-structure, I believe that a
> compound literal initializer would require additional braces for the
> initialization of its sub-objects too. At least I know that expressions
> like that have attracted the attention of clang people in the past:
> https://patchwork.ozlabs.org/project/netdev/patch/20190506202447.30907-1-natechancellor@gmail.com/
> So I went for the 'unambiguous' path.

There's a difference between:

	struct foo bar = { 0 };

and

	struct foo bar = { };

The former tells the compiler that you wish to set the first member of
struct foo, which will be an integer type, to zero. The latter is an
empty initialiser where all members and sub-members of the structure
default to a zero value.

You should have no problem with the latter. You will encounter problems
with the former if the first member of struct foo is not an integer
type.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge
  2021-08-10  9:12     ` Russell King - ARM Linux admin
@ 2021-08-10  9:34       ` Vladimir Oltean
  0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Oltean @ 2021-08-10  9:34 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Leon Romanovsky, netdev, Jakub Kicinski, David S. Miller,
	Andrew Lunn, Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Vadym Kochan, Taras Chornyi, Saeed Mahameed, Jiri Pirko,
	Ido Schimmel, Lars Povlsen, Steen Hegelund, UNGLinuxDriver,
	Grygorii Strashko, Julian Wiedmann, Karsten Graul,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger, Jianbo Liu,
	Vlad Buslov, Bjarni Jonasson, Vignesh Raghavendra,
	Tobias Waldekranz, linux-kernel, linux-rdma, linux-arm-kernel,
	linux-omap, linux-s390, Ido Schimmel

On Tue, Aug 10, 2021 at 10:12:38AM +0100, Russell King - ARM Linux admin wrote:
> There's a difference between:
> 
> 	struct foo bar = { 0 };
> 
> and
> 
> 	struct foo bar = { };
> 
> The former tells the compiler that you wish to set the first member of
> struct foo, which will be an integer type, to zero. The latter is an
> empty initialiser where all members and sub-members of the structure
> default to a zero value.
> 
> You should have no problem with the latter. You will encounter problems
> with the former if the first member of struct foo is not an integer
> type.

Ok, that's good to know. Seeing that this patch has not been applied yet
I'll go for a v2.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-10  9:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09 13:11 [PATCH net] net: switchdev: zero-initialize struct switchdev_notifier_fdb_info emitted by drivers towards the bridge Vladimir Oltean
2021-08-09 15:16 ` Ido Schimmel
2021-08-10  6:50 ` Leon Romanovsky
2021-08-10  8:16   ` Vladimir Oltean
2021-08-10  9:12     ` Russell King - ARM Linux admin
2021-08-10  9:34       ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).