[PATCH net-next v2 0/6] prestera: add basic router driver support

[PATCH net-next v2 0/6] prestera: add basic router driver support

[Yevhen Orlov]

Add initial router support for Marvell Prestera driver.
Subscribe on inetaddr notifications. TRAP packets, that has to be routed
(if packet has router's destination MAC address).

Add features:
 - Support ip address adding on port.
   e.g.: "ip address add PORT 1.1.1.1/24"

Limitations:
 - Only regular port supported. Vlan will be added soon.
 - It is routing through CPU. Offloading will be added in
   next patches.

Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>

Changes for v2:
* Remove useless assignment

Yevhen Orlov (6):
  net: marvell: prestera: add virtual router ABI
  net: marvell: prestera: Add router interface ABI
  net: marvell: prestera: Add prestera router infra
  net: marvell: prestera: add hardware router objects accounting
  net: marvell: prestera: Register inetaddr stub notifiers
  net: marvell: prestera: Implement initial inetaddr notifiers

 .../net/ethernet/marvell/prestera/Makefile    |   3 +-
 .../net/ethernet/marvell/prestera/prestera.h  |  38 ++++
 .../ethernet/marvell/prestera/prestera_hw.c   | 139 ++++++++++++
 .../ethernet/marvell/prestera/prestera_hw.h   |  11 +
 .../ethernet/marvell/prestera/prestera_main.c |   8 +-
 .../marvell/prestera/prestera_router.c        | 183 +++++++++++++++
 .../marvell/prestera/prestera_router_hw.c     | 209 ++++++++++++++++++
 .../marvell/prestera/prestera_router_hw.h     |  36 +++
 8 files changed, 625 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router.c
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router_hw.c
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router_hw.h

-- 
2.17.1

[PATCH net-next v2 1/6] net: marvell: prestera: add virtual router ABI

[Yevhen Orlov]

Add functions and structures to allocate virtual router.
prestera_hw_vr_create() return index of allocated VR so that we can move
forward and also add another objects (e.g. router interface),
which has link to VR.

Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
---
v1-->v2
* No changes
---
 .../ethernet/marvell/prestera/prestera_hw.c   | 42 +++++++++++++++++++
 .../ethernet/marvell/prestera/prestera_hw.h   |  4 ++
 2 files changed, 46 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 6282c9822e2b..8783adbad593 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -53,6 +53,9 @@ enum prestera_cmd_type_t {
 	PRESTERA_CMD_TYPE_VTCAM_IFACE_BIND = 0x560,
 	PRESTERA_CMD_TYPE_VTCAM_IFACE_UNBIND = 0x561,
 
+	PRESTERA_CMD_TYPE_ROUTER_VR_CREATE = 0x630,
+	PRESTERA_CMD_TYPE_ROUTER_VR_DELETE = 0x631,
+
 	PRESTERA_CMD_TYPE_RXTX_INIT = 0x800,
 
 	PRESTERA_CMD_TYPE_LAG_MEMBER_ADD = 0x900,
@@ -480,6 +483,18 @@ struct prestera_msg_rxtx_resp {
 	__le32 map_addr;
 };
 
+struct prestera_msg_vr_req {
+	struct prestera_msg_cmd cmd;
+	__le16 vr_id;
+	u8 __pad[2];
+};
+
+struct prestera_msg_vr_resp {
+	struct prestera_msg_ret ret;
+	__le16 vr_id;
+	u8 __pad[2];
+};
+
 struct prestera_msg_lag_req {
 	struct prestera_msg_cmd cmd;
 	__le32 port;
@@ -549,6 +564,7 @@ static void prestera_hw_build_tests(void)
 	BUILD_BUG_ON(sizeof(struct prestera_msg_acl_action) != 32);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_req) != 16);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_stats) != 16);
+	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_req) != 8);
 
 	/* check responses */
 	BUILD_BUG_ON(sizeof(struct prestera_msg_common_resp) != 8);
@@ -561,6 +577,7 @@ static void prestera_hw_build_tests(void)
 	BUILD_BUG_ON(sizeof(struct prestera_msg_rxtx_resp) != 12);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_vtcam_resp) != 16);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_resp) != 24);
+	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_resp) != 12);
 
 	/* check events */
 	BUILD_BUG_ON(sizeof(struct prestera_msg_event_port) != 20);
@@ -1752,6 +1769,31 @@ int prestera_hw_bridge_port_delete(struct prestera_port *port, u16 bridge_id)
 			    &req.cmd, sizeof(req));
 }
 
+int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id)
+{
+	int err;
+	struct prestera_msg_vr_resp resp;
+	struct prestera_msg_vr_req req;
+
+	err = prestera_cmd_ret(sw, PRESTERA_CMD_TYPE_ROUTER_VR_CREATE,
+			       &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
+	if (err)
+		return err;
+
+	*vr_id = __le16_to_cpu(resp.vr_id);
+	return err;
+}
+
+int prestera_hw_vr_delete(struct prestera_switch *sw, u16 vr_id)
+{
+	struct prestera_msg_vr_req req = {
+		.vr_id = __cpu_to_le16(vr_id),
+	};
+
+	return prestera_cmd(sw, PRESTERA_CMD_TYPE_ROUTER_VR_DELETE, &req.cmd,
+			    sizeof(req));
+}
+
 int prestera_hw_rxtx_init(struct prestera_switch *sw,
 			  struct prestera_rxtx_params *params)
 {
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
index 0496e454e148..6d9fafad451d 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
@@ -238,6 +238,10 @@ int prestera_hw_span_bind(const struct prestera_port *port, u8 span_id);
 int prestera_hw_span_unbind(const struct prestera_port *port);
 int prestera_hw_span_release(struct prestera_switch *sw, u8 span_id);
 
+/* Virtual Router API */
+int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id);
+int prestera_hw_vr_delete(struct prestera_switch *sw, u16 vr_id);
+
 /* Event handlers */
 int prestera_hw_event_handler_register(struct prestera_switch *sw,
 				       enum prestera_event_type type,
-- 
2.17.1

Re: [PATCH net-next v2 1/6] net: marvell: prestera: add virtual router ABI

[Ido Schimmel]

On Mon, Dec 27, 2021 at 11:52:26PM +0200, Yevhen Orlov wrote:
> Add functions and structures to allocate virtual router.
> prestera_hw_vr_create() return index of allocated VR so that we can move

s/return/returns/

> forward and also add another objects (e.g. router interface),
> which has link to VR.
> 
> Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
> ---
> v1-->v2
> * No changes
> ---
>  .../ethernet/marvell/prestera/prestera_hw.c   | 42 +++++++++++++++++++
>  .../ethernet/marvell/prestera/prestera_hw.h   |  4 ++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> index 6282c9822e2b..8783adbad593 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> @@ -53,6 +53,9 @@ enum prestera_cmd_type_t {
>  	PRESTERA_CMD_TYPE_VTCAM_IFACE_BIND = 0x560,
>  	PRESTERA_CMD_TYPE_VTCAM_IFACE_UNBIND = 0x561,
>  
> +	PRESTERA_CMD_TYPE_ROUTER_VR_CREATE = 0x630,
> +	PRESTERA_CMD_TYPE_ROUTER_VR_DELETE = 0x631,
> +
>  	PRESTERA_CMD_TYPE_RXTX_INIT = 0x800,
>  
>  	PRESTERA_CMD_TYPE_LAG_MEMBER_ADD = 0x900,
> @@ -480,6 +483,18 @@ struct prestera_msg_rxtx_resp {
>  	__le32 map_addr;
>  };
>  
> +struct prestera_msg_vr_req {
> +	struct prestera_msg_cmd cmd;
> +	__le16 vr_id;
> +	u8 __pad[2];
> +};
> +
> +struct prestera_msg_vr_resp {
> +	struct prestera_msg_ret ret;
> +	__le16 vr_id;
> +	u8 __pad[2];
> +};
> +
>  struct prestera_msg_lag_req {
>  	struct prestera_msg_cmd cmd;
>  	__le32 port;
> @@ -549,6 +564,7 @@ static void prestera_hw_build_tests(void)
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_acl_action) != 32);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_req) != 16);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_stats) != 16);
> +	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_req) != 8);
>  
>  	/* check responses */
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_common_resp) != 8);
> @@ -561,6 +577,7 @@ static void prestera_hw_build_tests(void)
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_rxtx_resp) != 12);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_vtcam_resp) != 16);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_resp) != 24);
> +	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_resp) != 12);
>  
>  	/* check events */
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_event_port) != 20);
> @@ -1752,6 +1769,31 @@ int prestera_hw_bridge_port_delete(struct prestera_port *port, u16 bridge_id)
>  			    &req.cmd, sizeof(req));
>  }
>  
> +int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id)
> +{
> +	int err;
> +	struct prestera_msg_vr_resp resp;
> +	struct prestera_msg_vr_req req;

Order local variables from longest to shortest (reverse xmas tree), so
'int err' should be at the end. Same in other places I might have missed

> +
> +	err = prestera_cmd_ret(sw, PRESTERA_CMD_TYPE_ROUTER_VR_CREATE,
> +			       &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
> +	if (err)
> +		return err;
> +
> +	*vr_id = __le16_to_cpu(resp.vr_id);
> +	return err;
> +}
> +
> +int prestera_hw_vr_delete(struct prestera_switch *sw, u16 vr_id)
> +{
> +	struct prestera_msg_vr_req req = {
> +		.vr_id = __cpu_to_le16(vr_id),
> +	};
> +
> +	return prestera_cmd(sw, PRESTERA_CMD_TYPE_ROUTER_VR_DELETE, &req.cmd,
> +			    sizeof(req));
> +}
> +
>  int prestera_hw_rxtx_init(struct prestera_switch *sw,
>  			  struct prestera_rxtx_params *params)
>  {
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> index 0496e454e148..6d9fafad451d 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> @@ -238,6 +238,10 @@ int prestera_hw_span_bind(const struct prestera_port *port, u8 span_id);
>  int prestera_hw_span_unbind(const struct prestera_port *port);
>  int prestera_hw_span_release(struct prestera_switch *sw, u8 span_id);
>  
> +/* Virtual Router API */
> +int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id);
> +int prestera_hw_vr_delete(struct prestera_switch *sw, u16 vr_id);
> +
>  /* Event handlers */
>  int prestera_hw_event_handler_register(struct prestera_switch *sw,
>  				       enum prestera_event_type type,
> -- 
> 2.17.1
> 

[PATCH net-next v2 2/6] net: marvell: prestera: Add router interface ABI

[Yevhen Orlov]

Add functions to enable routing on port, which is not in vlan.
Also we can enable routing on vlan.
prestera_hw_rif_create() take index of allocated virtual router.

Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
---
v1-->v2
* No changes
---
 .../net/ethernet/marvell/prestera/prestera.h  | 23 +++++
 .../ethernet/marvell/prestera/prestera_hw.c   | 97 +++++++++++++++++++
 .../ethernet/marvell/prestera/prestera_hw.h   |  7 ++
 3 files changed, 127 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
index 797b2e4d3551..636caf492531 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera.h
@@ -225,6 +225,29 @@ struct prestera_event {
 	};
 };
 
+enum prestera_if_type {
+	/* the interface is of port type (dev,port) */
+	PRESTERA_IF_PORT_E = 0,
+
+	/* the interface is of lag type (lag-id) */
+	PRESTERA_IF_LAG_E = 1,
+
+	/* the interface is of Vid type (vlan-id) */
+	PRESTERA_IF_VID_E = 3,
+};
+
+struct prestera_iface {
+	enum prestera_if_type type;
+	struct {
+		u32 hw_dev_num;
+		u32 port_num;
+	} dev_port;
+	u32 hw_dev_num;
+	u16 vr_id;
+	u16 lag_id;
+	u16 vlan_id;
+};
+
 struct prestera_switchdev;
 struct prestera_span;
 struct prestera_rxtx;
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 8783adbad593..51fc841b1e7a 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -53,6 +53,8 @@ enum prestera_cmd_type_t {
 	PRESTERA_CMD_TYPE_VTCAM_IFACE_BIND = 0x560,
 	PRESTERA_CMD_TYPE_VTCAM_IFACE_UNBIND = 0x561,
 
+	PRESTERA_CMD_TYPE_ROUTER_RIF_CREATE = 0x600,
+	PRESTERA_CMD_TYPE_ROUTER_RIF_DELETE = 0x601,
 	PRESTERA_CMD_TYPE_ROUTER_VR_CREATE = 0x630,
 	PRESTERA_CMD_TYPE_ROUTER_VR_DELETE = 0x631,
 
@@ -483,6 +485,36 @@ struct prestera_msg_rxtx_resp {
 	__le32 map_addr;
 };
 
+struct prestera_msg_iface {
+	union {
+		struct {
+			__le32 dev;
+			__le32 port;
+		};
+		__le16 lag_id;
+	};
+	__le16 vr_id;
+	__le16 vid;
+	u8 type;
+	u8 __pad[3];
+};
+
+struct prestera_msg_rif_req {
+	struct prestera_msg_cmd cmd;
+	struct prestera_msg_iface iif;
+	__le32 mtu;
+	__le16 rif_id;
+	__le16 __reserved;
+	u8 mac[ETH_ALEN];
+	u8 __pad[2];
+};
+
+struct prestera_msg_rif_resp {
+	struct prestera_msg_ret ret;
+	__le16 rif_id;
+	u8 __pad[2];
+};
+
 struct prestera_msg_vr_req {
 	struct prestera_msg_cmd cmd;
 	__le16 vr_id;
@@ -564,8 +596,12 @@ static void prestera_hw_build_tests(void)
 	BUILD_BUG_ON(sizeof(struct prestera_msg_acl_action) != 32);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_req) != 16);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_stats) != 16);
+	BUILD_BUG_ON(sizeof(struct prestera_msg_rif_req) != 36);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_req) != 8);
 
+	/*  structure that are part of req/resp fw messages */
+	BUILD_BUG_ON(sizeof(struct prestera_msg_iface) != 16);
+
 	/* check responses */
 	BUILD_BUG_ON(sizeof(struct prestera_msg_common_resp) != 8);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_switch_init_resp) != 24);
@@ -577,6 +613,7 @@ static void prestera_hw_build_tests(void)
 	BUILD_BUG_ON(sizeof(struct prestera_msg_rxtx_resp) != 12);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_vtcam_resp) != 16);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_resp) != 24);
+	BUILD_BUG_ON(sizeof(struct prestera_msg_rif_resp) != 12);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_resp) != 12);
 
 	/* check events */
@@ -1769,6 +1806,66 @@ int prestera_hw_bridge_port_delete(struct prestera_port *port, u16 bridge_id)
 			    &req.cmd, sizeof(req));
 }
 
+static int prestera_iface_to_msg(struct prestera_iface *iface,
+				 struct prestera_msg_iface *msg_if)
+{
+	switch (iface->type) {
+	case PRESTERA_IF_PORT_E:
+	case PRESTERA_IF_VID_E:
+		msg_if->port = __cpu_to_le32(iface->dev_port.port_num);
+		msg_if->dev = __cpu_to_le32(iface->dev_port.hw_dev_num);
+		break;
+	case PRESTERA_IF_LAG_E:
+		msg_if->lag_id = __cpu_to_le16(iface->lag_id);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	msg_if->vr_id = __cpu_to_le16(iface->vr_id);
+	msg_if->vid = __cpu_to_le16(iface->vlan_id);
+	msg_if->type = iface->type;
+	return 0;
+}
+
+int prestera_hw_rif_create(struct prestera_switch *sw,
+			   struct prestera_iface *iif, u8 *mac, u16 *rif_id)
+{
+	struct prestera_msg_rif_req req;
+	struct prestera_msg_rif_resp resp;
+	int err;
+
+	memcpy(req.mac, mac, ETH_ALEN);
+
+	err = prestera_iface_to_msg(iif, &req.iif);
+	if (err)
+		return err;
+
+	err = prestera_cmd_ret(sw, PRESTERA_CMD_TYPE_ROUTER_RIF_CREATE,
+			       &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
+	if (err)
+		return err;
+
+	*rif_id = __le16_to_cpu(resp.rif_id);
+	return err;
+}
+
+int prestera_hw_rif_delete(struct prestera_switch *sw, u16 rif_id,
+			   struct prestera_iface *iif)
+{
+	struct prestera_msg_rif_req req = {
+		.rif_id = __cpu_to_le16(rif_id),
+	};
+	int err;
+
+	err = prestera_iface_to_msg(iif, &req.iif);
+	if (err)
+		return err;
+
+	return prestera_cmd(sw, PRESTERA_CMD_TYPE_ROUTER_RIF_DELETE, &req.cmd,
+			    sizeof(req));
+}
+
 int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id)
 {
 	int err;
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
index 6d9fafad451d..3ff12bae5909 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
@@ -137,6 +137,7 @@ struct prestera_rxtx_params;
 struct prestera_acl_hw_action_info;
 struct prestera_acl_iface;
 struct prestera_counter_stats;
+struct prestera_iface;
 
 /* Switch API */
 int prestera_hw_switch_init(struct prestera_switch *sw);
@@ -238,6 +239,12 @@ int prestera_hw_span_bind(const struct prestera_port *port, u8 span_id);
 int prestera_hw_span_unbind(const struct prestera_port *port);
 int prestera_hw_span_release(struct prestera_switch *sw, u8 span_id);
 
+/* Router API */
+int prestera_hw_rif_create(struct prestera_switch *sw,
+			   struct prestera_iface *iif, u8 *mac, u16 *rif_id);
+int prestera_hw_rif_delete(struct prestera_switch *sw, u16 rif_id,
+			   struct prestera_iface *iif);
+
 /* Virtual Router API */
 int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id);
 int prestera_hw_vr_delete(struct prestera_switch *sw, u16 vr_id);
-- 
2.17.1

Re: [PATCH net-next v2 2/6] net: marvell: prestera: Add router interface ABI

[Ido Schimmel]

On Mon, Dec 27, 2021 at 11:52:27PM +0200, Yevhen Orlov wrote:
> Add functions to enable routing on port, which is not in vlan.
> Also we can enable routing on vlan.

I don't understand these two lines. Can you explain for which netdev
types you can create a router interface?

> prestera_hw_rif_create() take index of allocated virtual router.

s/take/takes/

> 
> Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
> ---
> v1-->v2
> * No changes
> ---
>  .../net/ethernet/marvell/prestera/prestera.h  | 23 +++++
>  .../ethernet/marvell/prestera/prestera_hw.c   | 97 +++++++++++++++++++
>  .../ethernet/marvell/prestera/prestera_hw.h   |  7 ++
>  3 files changed, 127 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
> index 797b2e4d3551..636caf492531 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera.h
> @@ -225,6 +225,29 @@ struct prestera_event {
>  	};
>  };
>  
> +enum prestera_if_type {
> +	/* the interface is of port type (dev,port) */
> +	PRESTERA_IF_PORT_E = 0,
> +
> +	/* the interface is of lag type (lag-id) */
> +	PRESTERA_IF_LAG_E = 1,
> +
> +	/* the interface is of Vid type (vlan-id) */
> +	PRESTERA_IF_VID_E = 3,
> +};
> +
> +struct prestera_iface {
> +	enum prestera_if_type type;
> +	struct {
> +		u32 hw_dev_num;
> +		u32 port_num;
> +	} dev_port;
> +	u32 hw_dev_num;
> +	u16 vr_id;
> +	u16 lag_id;
> +	u16 vlan_id;
> +};
> +
>  struct prestera_switchdev;
>  struct prestera_span;
>  struct prestera_rxtx;
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> index 8783adbad593..51fc841b1e7a 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> @@ -53,6 +53,8 @@ enum prestera_cmd_type_t {
>  	PRESTERA_CMD_TYPE_VTCAM_IFACE_BIND = 0x560,
>  	PRESTERA_CMD_TYPE_VTCAM_IFACE_UNBIND = 0x561,
>  
> +	PRESTERA_CMD_TYPE_ROUTER_RIF_CREATE = 0x600,
> +	PRESTERA_CMD_TYPE_ROUTER_RIF_DELETE = 0x601,
>  	PRESTERA_CMD_TYPE_ROUTER_VR_CREATE = 0x630,
>  	PRESTERA_CMD_TYPE_ROUTER_VR_DELETE = 0x631,
>  
> @@ -483,6 +485,36 @@ struct prestera_msg_rxtx_resp {
>  	__le32 map_addr;
>  };
>  
> +struct prestera_msg_iface {
> +	union {
> +		struct {
> +			__le32 dev;
> +			__le32 port;
> +		};
> +		__le16 lag_id;
> +	};
> +	__le16 vr_id;
> +	__le16 vid;
> +	u8 type;
> +	u8 __pad[3];
> +};
> +
> +struct prestera_msg_rif_req {
> +	struct prestera_msg_cmd cmd;
> +	struct prestera_msg_iface iif;
> +	__le32 mtu;
> +	__le16 rif_id;
> +	__le16 __reserved;
> +	u8 mac[ETH_ALEN];
> +	u8 __pad[2];
> +};
> +
> +struct prestera_msg_rif_resp {
> +	struct prestera_msg_ret ret;
> +	__le16 rif_id;
> +	u8 __pad[2];
> +};
> +
>  struct prestera_msg_vr_req {
>  	struct prestera_msg_cmd cmd;
>  	__le16 vr_id;
> @@ -564,8 +596,12 @@ static void prestera_hw_build_tests(void)
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_acl_action) != 32);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_req) != 16);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_stats) != 16);
> +	BUILD_BUG_ON(sizeof(struct prestera_msg_rif_req) != 36);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_req) != 8);
>  
> +	/*  structure that are part of req/resp fw messages */
> +	BUILD_BUG_ON(sizeof(struct prestera_msg_iface) != 16);
> +
>  	/* check responses */
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_common_resp) != 8);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_switch_init_resp) != 24);
> @@ -577,6 +613,7 @@ static void prestera_hw_build_tests(void)
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_rxtx_resp) != 12);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_vtcam_resp) != 16);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_counter_resp) != 24);
> +	BUILD_BUG_ON(sizeof(struct prestera_msg_rif_resp) != 12);
>  	BUILD_BUG_ON(sizeof(struct prestera_msg_vr_resp) != 12);
>  
>  	/* check events */
> @@ -1769,6 +1806,66 @@ int prestera_hw_bridge_port_delete(struct prestera_port *port, u16 bridge_id)
>  			    &req.cmd, sizeof(req));
>  }
>  
> +static int prestera_iface_to_msg(struct prestera_iface *iface,
> +				 struct prestera_msg_iface *msg_if)
> +{
> +	switch (iface->type) {
> +	case PRESTERA_IF_PORT_E:
> +	case PRESTERA_IF_VID_E:
> +		msg_if->port = __cpu_to_le32(iface->dev_port.port_num);
> +		msg_if->dev = __cpu_to_le32(iface->dev_port.hw_dev_num);
> +		break;
> +	case PRESTERA_IF_LAG_E:
> +		msg_if->lag_id = __cpu_to_le16(iface->lag_id);
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	msg_if->vr_id = __cpu_to_le16(iface->vr_id);
> +	msg_if->vid = __cpu_to_le16(iface->vlan_id);
> +	msg_if->type = iface->type;
> +	return 0;
> +}
> +
> +int prestera_hw_rif_create(struct prestera_switch *sw,
> +			   struct prestera_iface *iif, u8 *mac, u16 *rif_id)
> +{
> +	struct prestera_msg_rif_req req;
> +	struct prestera_msg_rif_resp resp;
> +	int err;

Same comment as before

> +
> +	memcpy(req.mac, mac, ETH_ALEN);

Each RIF can use whatever MAC it wants? You don't have limitations on
common prefix or something like that? Guess it depends on how many RIFs
you can support

> +
> +	err = prestera_iface_to_msg(iif, &req.iif);
> +	if (err)
> +		return err;
> +
> +	err = prestera_cmd_ret(sw, PRESTERA_CMD_TYPE_ROUTER_RIF_CREATE,
> +			       &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
> +	if (err)
> +		return err;
> +
> +	*rif_id = __le16_to_cpu(resp.rif_id);
> +	return err;
> +}
> +
> +int prestera_hw_rif_delete(struct prestera_switch *sw, u16 rif_id,
> +			   struct prestera_iface *iif)
> +{
> +	struct prestera_msg_rif_req req = {
> +		.rif_id = __cpu_to_le16(rif_id),
> +	};
> +	int err;
> +
> +	err = prestera_iface_to_msg(iif, &req.iif);
> +	if (err)
> +		return err;
> +
> +	return prestera_cmd(sw, PRESTERA_CMD_TYPE_ROUTER_RIF_DELETE, &req.cmd,
> +			    sizeof(req));
> +}
> +
>  int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id)
>  {
>  	int err;
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> index 6d9fafad451d..3ff12bae5909 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> @@ -137,6 +137,7 @@ struct prestera_rxtx_params;
>  struct prestera_acl_hw_action_info;
>  struct prestera_acl_iface;
>  struct prestera_counter_stats;
> +struct prestera_iface;
>  
>  /* Switch API */
>  int prestera_hw_switch_init(struct prestera_switch *sw);
> @@ -238,6 +239,12 @@ int prestera_hw_span_bind(const struct prestera_port *port, u8 span_id);
>  int prestera_hw_span_unbind(const struct prestera_port *port);
>  int prestera_hw_span_release(struct prestera_switch *sw, u8 span_id);
>  
> +/* Router API */
> +int prestera_hw_rif_create(struct prestera_switch *sw,
> +			   struct prestera_iface *iif, u8 *mac, u16 *rif_id);
> +int prestera_hw_rif_delete(struct prestera_switch *sw, u16 rif_id,
> +			   struct prestera_iface *iif);
> +
>  /* Virtual Router API */
>  int prestera_hw_vr_create(struct prestera_switch *sw, u16 *vr_id);
>  int prestera_hw_vr_delete(struct prestera_switch *sw, u16 vr_id);
> -- 
> 2.17.1
> 

Re: [PATCH net-next v2 2/6] net: marvell: prestera: Add router interface ABI

[Yevhen Orlov]

On Thu, Dec 30, 2021 at 03:44:50PM +0200, Ido Schimmel wrote:
> On Mon, Dec 27, 2021 at 11:52:27PM +0200, Yevhen Orlov wrote:
> > Add functions to enable routing on port, which is not in vlan.
> > Also we can enable routing on vlan.
>
> I don't understand these two lines. Can you explain for which netdev
> types you can create a router interface?
>

Sure.
For now we support only regular port (has no upper or lower dev).

But ABI potentially support RIF on bridge/vlan (see prestera_if_type).
This feature will be implemented soon.

[PATCH net-next v2 3/6] net: marvell: prestera: Add prestera router infra

[Yevhen Orlov]

Add prestera_router.c, which contains code to subscribe/unsubscribe on
kernel notifiers for router. This handle kernel notifications,
parse structures to make key to manipulate prestera_router_hw's objects.

Also prestera_router is container for router's objects database.

Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
---
v1-->v2
* No changes
---
 .../net/ethernet/marvell/prestera/Makefile    |  3 +-
 .../net/ethernet/marvell/prestera/prestera.h  | 11 ++++++++
 .../ethernet/marvell/prestera/prestera_main.c |  6 ++++
 .../marvell/prestera/prestera_router.c        | 28 +++++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router.c

diff --git a/drivers/net/ethernet/marvell/prestera/Makefile b/drivers/net/ethernet/marvell/prestera/Makefile
index 48dbcb2baf8f..ec69fc564a9f 100644
--- a/drivers/net/ethernet/marvell/prestera/Makefile
+++ b/drivers/net/ethernet/marvell/prestera/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PRESTERA)	+= prestera.o
 prestera-objs		:= prestera_main.o prestera_hw.o prestera_dsa.o \
 			   prestera_rxtx.o prestera_devlink.o prestera_ethtool.o \
 			   prestera_switchdev.o prestera_acl.o prestera_flow.o \
-			   prestera_flower.o prestera_span.o prestera_counter.o
+			   prestera_flower.o prestera_span.o prestera_counter.o \
+			   prestera_router.o
 
 obj-$(CONFIG_PRESTERA_PCI)	+= prestera_pci.o
diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
index 636caf492531..7160da678457 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera.h
@@ -270,12 +270,20 @@ struct prestera_switch {
 	u32 mtu_min;
 	u32 mtu_max;
 	u8 id;
+	struct prestera_router *router;
 	struct prestera_lag *lags;
 	struct prestera_counter *counter;
 	u8 lag_member_max;
 	u8 lag_max;
 };
 
+struct prestera_router {
+	struct prestera_switch *sw;
+	struct list_head vr_list;
+	struct list_head rif_entry_list;
+	bool aborted;
+};
+
 struct prestera_rxtx_params {
 	bool use_sdma;
 	u32 map_addr;
@@ -303,6 +311,9 @@ struct prestera_port *prestera_port_find_by_hwid(struct prestera_switch *sw,
 
 int prestera_port_autoneg_set(struct prestera_port *port, u64 link_modes);
 
+int prestera_router_init(struct prestera_switch *sw);
+void prestera_router_fini(struct prestera_switch *sw);
+
 struct prestera_port *prestera_find_port(struct prestera_switch *sw, u32 id);
 
 int prestera_port_cfg_mac_read(struct prestera_port *port,
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index a0dbad5cb88d..242904fcd866 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -893,6 +893,10 @@ static int prestera_switch_init(struct prestera_switch *sw)
 	if (err)
 		return err;
 
+	err = prestera_router_init(sw);
+	if (err)
+		goto err_router_init;
+
 	err = prestera_switchdev_init(sw);
 	if (err)
 		goto err_swdev_register;
@@ -949,6 +953,8 @@ static int prestera_switch_init(struct prestera_switch *sw)
 err_rxtx_register:
 	prestera_switchdev_fini(sw);
 err_swdev_register:
+	prestera_router_fini(sw);
+err_router_init:
 	prestera_netdev_event_handler_unregister(sw);
 	prestera_hw_switch_fini(sw);
 
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
new file mode 100644
index 000000000000..f3980d10eb29
--- /dev/null
+++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2019-2021 Marvell International Ltd. All rights reserved */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+
+#include "prestera.h"
+
+int prestera_router_init(struct prestera_switch *sw)
+{
+	struct prestera_router *router;
+
+	router = kzalloc(sizeof(*sw->router), GFP_KERNEL);
+	if (!router)
+		return -ENOMEM;
+
+	sw->router = router;
+	router->sw = sw;
+
+	return 0;
+}
+
+void prestera_router_fini(struct prestera_switch *sw)
+{
+	kfree(sw->router);
+	sw->router = NULL;
+}
+
-- 
2.17.1

Re: [PATCH net-next v2 3/6] net: marvell: prestera: Add prestera router infra

[Ido Schimmel]

On Mon, Dec 27, 2021 at 11:52:28PM +0200, Yevhen Orlov wrote:
> Add prestera_router.c, which contains code to subscribe/unsubscribe on
> kernel notifiers for router. This handle kernel notifications,
> parse structures to make key to manipulate prestera_router_hw's objects.
> 
> Also prestera_router is container for router's objects database.
> 
> Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
> ---
> v1-->v2
> * No changes
> ---
>  .../net/ethernet/marvell/prestera/Makefile    |  3 +-
>  .../net/ethernet/marvell/prestera/prestera.h  | 11 ++++++++
>  .../ethernet/marvell/prestera/prestera_main.c |  6 ++++
>  .../marvell/prestera/prestera_router.c        | 28 +++++++++++++++++++
>  4 files changed, 47 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router.c
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/Makefile b/drivers/net/ethernet/marvell/prestera/Makefile
> index 48dbcb2baf8f..ec69fc564a9f 100644
> --- a/drivers/net/ethernet/marvell/prestera/Makefile
> +++ b/drivers/net/ethernet/marvell/prestera/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_PRESTERA)	+= prestera.o
>  prestera-objs		:= prestera_main.o prestera_hw.o prestera_dsa.o \
>  			   prestera_rxtx.o prestera_devlink.o prestera_ethtool.o \
>  			   prestera_switchdev.o prestera_acl.o prestera_flow.o \
> -			   prestera_flower.o prestera_span.o prestera_counter.o
> +			   prestera_flower.o prestera_span.o prestera_counter.o \
> +			   prestera_router.o
>  
>  obj-$(CONFIG_PRESTERA_PCI)	+= prestera_pci.o
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
> index 636caf492531..7160da678457 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera.h
> @@ -270,12 +270,20 @@ struct prestera_switch {
>  	u32 mtu_min;
>  	u32 mtu_max;
>  	u8 id;
> +	struct prestera_router *router;
>  	struct prestera_lag *lags;
>  	struct prestera_counter *counter;
>  	u8 lag_member_max;
>  	u8 lag_max;
>  };
>  
> +struct prestera_router {
> +	struct prestera_switch *sw;
> +	struct list_head vr_list;
> +	struct list_head rif_entry_list;
> +	bool aborted;

Never used

> +};
> +
>  struct prestera_rxtx_params {
>  	bool use_sdma;
>  	u32 map_addr;
> @@ -303,6 +311,9 @@ struct prestera_port *prestera_port_find_by_hwid(struct prestera_switch *sw,
>  
>  int prestera_port_autoneg_set(struct prestera_port *port, u64 link_modes);
>  
> +int prestera_router_init(struct prestera_switch *sw);
> +void prestera_router_fini(struct prestera_switch *sw);
> +
>  struct prestera_port *prestera_find_port(struct prestera_switch *sw, u32 id);
>  
>  int prestera_port_cfg_mac_read(struct prestera_port *port,
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> index a0dbad5cb88d..242904fcd866 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> @@ -893,6 +893,10 @@ static int prestera_switch_init(struct prestera_switch *sw)
>  	if (err)
>  		return err;
>  
> +	err = prestera_router_init(sw);
> +	if (err)
> +		goto err_router_init;
> +
>  	err = prestera_switchdev_init(sw);
>  	if (err)
>  		goto err_swdev_register;
> @@ -949,6 +953,8 @@ static int prestera_switch_init(struct prestera_switch *sw)
>  err_rxtx_register:
>  	prestera_switchdev_fini(sw);
>  err_swdev_register:
> +	prestera_router_fini(sw);

Missing a call in prestera_switch_fini(). Most likely visible with
kmemleak enabled

> +err_router_init:
>  	prestera_netdev_event_handler_unregister(sw);
>  	prestera_hw_switch_fini(sw);
>  
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> new file mode 100644
> index 000000000000..f3980d10eb29
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> +/* Copyright (c) 2019-2021 Marvell International Ltd. All rights reserved */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +
> +#include "prestera.h"
> +
> +int prestera_router_init(struct prestera_switch *sw)
> +{
> +	struct prestera_router *router;
> +
> +	router = kzalloc(sizeof(*sw->router), GFP_KERNEL);
> +	if (!router)
> +		return -ENOMEM;
> +
> +	sw->router = router;
> +	router->sw = sw;
> +
> +	return 0;
> +}
> +
> +void prestera_router_fini(struct prestera_switch *sw)
> +{
> +	kfree(sw->router);
> +	sw->router = NULL;
> +}
> +
> -- 
> 2.17.1
> 

[PATCH net-next v2 4/6] net: marvell: prestera: add hardware router objects accounting

[Yevhen Orlov]

Add prestera_router_hw.c. This file contains functions, which track HW
objects relations and links. This include implicity creation of objects,
that needed by requested one and implicity removing of objects, which
reference counter is became zero.

We need this layer, because kernel callbacks not always mapped to
creation of single HW object. So let it be two different layers - one
for subscribing and parsing kernel structures, and another
(prestera_router_hw.c) for HW objects relations tracking.

There is two types of objects on router_hw layer:
 - Explicit objects (rif_entry) : created by higher layer.
 - Implicit objects (vr) : created on demand by explicit objects.

Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
---
v1-->v2
* No changes
---
 .../net/ethernet/marvell/prestera/Makefile    |   2 +-
 .../marvell/prestera/prestera_router.c        |  10 +
 .../marvell/prestera/prestera_router_hw.c     | 209 ++++++++++++++++++
 .../marvell/prestera/prestera_router_hw.h     |  36 +++
 4 files changed, 256 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router_hw.c
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router_hw.h

diff --git a/drivers/net/ethernet/marvell/prestera/Makefile b/drivers/net/ethernet/marvell/prestera/Makefile
index ec69fc564a9f..d395f4131648 100644
--- a/drivers/net/ethernet/marvell/prestera/Makefile
+++ b/drivers/net/ethernet/marvell/prestera/Makefile
@@ -4,6 +4,6 @@ prestera-objs		:= prestera_main.o prestera_hw.o prestera_dsa.o \
 			   prestera_rxtx.o prestera_devlink.o prestera_ethtool.o \
 			   prestera_switchdev.o prestera_acl.o prestera_flow.o \
 			   prestera_flower.o prestera_span.o prestera_counter.o \
-			   prestera_router.o
+			   prestera_router.o prestera_router_hw.o
 
 obj-$(CONFIG_PRESTERA_PCI)	+= prestera_pci.o
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
index f3980d10eb29..2a32831df40f 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
@@ -5,10 +5,12 @@
 #include <linux/types.h>
 
 #include "prestera.h"
+#include "prestera_router_hw.h"
 
 int prestera_router_init(struct prestera_switch *sw)
 {
 	struct prestera_router *router;
+	int err;
 
 	router = kzalloc(sizeof(*sw->router), GFP_KERNEL);
 	if (!router)
@@ -17,7 +19,15 @@ int prestera_router_init(struct prestera_switch *sw)
 	sw->router = router;
 	router->sw = sw;
 
+	err = prestera_router_hw_init(sw);
+	if (err)
+		goto err_router_lib_init;
+
 	return 0;
+
+err_router_lib_init:
+	kfree(sw->router);
+	return err;
 }
 
 void prestera_router_fini(struct prestera_switch *sw)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.c
new file mode 100644
index 000000000000..4f66fb21a299
--- /dev/null
+++ b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2019-2021 Marvell International Ltd. All rights reserved */
+
+#include <linux/rhashtable.h>
+
+#include "prestera.h"
+#include "prestera_hw.h"
+#include "prestera_router_hw.h"
+#include "prestera_acl.h"
+
+/*            +--+
+ *   +------->|vr|
+ *   |        +--+
+ *   |
+ * +-+-------+
+ * |rif_entry|
+ * +---------+
+ *  Rif is
+ *  used as
+ *  entry point
+ *  for vr in hw
+ */
+
+int prestera_router_hw_init(struct prestera_switch *sw)
+{
+	INIT_LIST_HEAD(&sw->router->vr_list);
+	INIT_LIST_HEAD(&sw->router->rif_entry_list);
+
+	return 0;
+}
+
+static struct prestera_vr *__prestera_vr_find(struct prestera_switch *sw,
+					      u32 tb_id)
+{
+	struct prestera_vr *vr;
+
+	list_for_each_entry(vr, &sw->router->vr_list, router_node) {
+		if (vr->tb_id == tb_id)
+			return vr;
+	}
+
+	return NULL;
+}
+
+static struct prestera_vr *__prestera_vr_create(struct prestera_switch *sw,
+						u32 tb_id,
+						struct netlink_ext_ack *extack)
+{
+	struct prestera_vr *vr;
+	u16 hw_vr_id;
+	int err;
+
+	err = prestera_hw_vr_create(sw, &hw_vr_id);
+	if (err)
+		return ERR_PTR(-ENOMEM);
+
+	vr = kzalloc(sizeof(*vr), GFP_KERNEL);
+	if (!vr) {
+		err = -ENOMEM;
+		goto err_alloc_vr;
+	}
+
+	vr->tb_id = tb_id;
+	vr->hw_vr_id = hw_vr_id;
+
+	list_add(&vr->router_node, &sw->router->vr_list);
+
+	return vr;
+
+err_alloc_vr:
+	prestera_hw_vr_delete(sw, hw_vr_id);
+	kfree(vr);
+	return ERR_PTR(err);
+}
+
+static void __prestera_vr_destroy(struct prestera_switch *sw,
+				  struct prestera_vr *vr)
+{
+	prestera_hw_vr_delete(sw, vr->hw_vr_id);
+	list_del(&vr->router_node);
+	kfree(vr);
+}
+
+static struct prestera_vr *prestera_vr_get(struct prestera_switch *sw, u32 tb_id,
+					   struct netlink_ext_ack *extack)
+{
+	struct prestera_vr *vr;
+
+	vr = __prestera_vr_find(sw, tb_id);
+	if (!vr)
+		vr = __prestera_vr_create(sw, tb_id, extack);
+	if (IS_ERR(vr))
+		return ERR_CAST(vr);
+
+	return vr;
+}
+
+static void prestera_vr_put(struct prestera_switch *sw, struct prestera_vr *vr)
+{
+	if (!vr->ref_cnt)
+		__prestera_vr_destroy(sw, vr);
+}
+
+/* iface is overhead struct. vr_id also can be removed. */
+static int
+__prestera_rif_entry_key_copy(const struct prestera_rif_entry_key *in,
+			      struct prestera_rif_entry_key *out)
+{
+	memset(out, 0, sizeof(*out));
+
+	switch (in->iface.type) {
+	case PRESTERA_IF_PORT_E:
+		out->iface.dev_port.hw_dev_num = in->iface.dev_port.hw_dev_num;
+		out->iface.dev_port.port_num = in->iface.dev_port.port_num;
+		break;
+	case PRESTERA_IF_LAG_E:
+		out->iface.lag_id = in->iface.lag_id;
+		break;
+	case PRESTERA_IF_VID_E:
+		out->iface.vlan_id = in->iface.vlan_id;
+		break;
+	default:
+		pr_err("Unsupported iface type");
+		return -EINVAL;
+	}
+
+	out->iface.type = in->iface.type;
+	return 0;
+}
+
+struct prestera_rif_entry *
+prestera_rif_entry_find(const struct prestera_switch *sw,
+			const struct prestera_rif_entry_key *k)
+{
+	struct prestera_rif_entry *rif_entry;
+	struct prestera_rif_entry_key lk; /* lookup key */
+
+	if (__prestera_rif_entry_key_copy(k, &lk))
+		return NULL;
+
+	list_for_each_entry(rif_entry, &sw->router->rif_entry_list,
+			    router_node) {
+		if (!memcmp(k, &rif_entry->key, sizeof(*k)))
+			return rif_entry;
+	}
+
+	return NULL;
+}
+
+void prestera_rif_entry_destroy(struct prestera_switch *sw,
+				struct prestera_rif_entry *e)
+{
+	struct prestera_iface iface;
+
+	list_del(&e->router_node);
+
+	memcpy(&iface, &e->key.iface, sizeof(iface));
+	iface.vr_id = e->vr->hw_vr_id;
+	prestera_hw_rif_delete(sw, e->hw_id, &iface);
+
+	e->vr->ref_cnt--;
+	prestera_vr_put(sw, e->vr);
+	kfree(e);
+}
+
+struct prestera_rif_entry *
+prestera_rif_entry_create(struct prestera_switch *sw,
+			  struct prestera_rif_entry_key *k,
+			  u32 tb_id, const unsigned char *addr)
+{
+	int err;
+	struct prestera_rif_entry *e;
+	struct prestera_iface iface;
+
+	e = kzalloc(sizeof(*e), GFP_KERNEL);
+	if (!e)
+		goto err_kzalloc;
+
+	if (__prestera_rif_entry_key_copy(k, &e->key))
+		goto err_key_copy;
+
+	e->vr = prestera_vr_get(sw, tb_id, NULL);
+	if (IS_ERR(e->vr))
+		goto err_vr_get;
+
+	e->vr->ref_cnt++;
+	memcpy(&e->addr, addr, sizeof(e->addr));
+
+	/* HW */
+	memcpy(&iface, &e->key.iface, sizeof(iface));
+	iface.vr_id = e->vr->hw_vr_id;
+	err = prestera_hw_rif_create(sw, &iface, e->addr, &e->hw_id);
+	if (err)
+		goto err_hw_create;
+
+	list_add(&e->router_node, &sw->router->rif_entry_list);
+
+	return e;
+
+err_hw_create:
+	e->vr->ref_cnt--;
+	prestera_vr_put(sw, e->vr);
+err_vr_get:
+err_key_copy:
+	kfree(e);
+err_kzalloc:
+	return NULL;
+}
+
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.h
new file mode 100644
index 000000000000..fed53595f7bb
--- /dev/null
+++ b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2019-2021 Marvell International Ltd. All rights reserved. */
+
+#ifndef _PRESTERA_ROUTER_HW_H_
+#define _PRESTERA_ROUTER_HW_H_
+
+struct prestera_vr {
+	struct list_head router_node;
+	unsigned int ref_cnt;
+	u32 tb_id;			/* key (kernel fib table id) */
+	u16 hw_vr_id;			/* virtual router ID */
+	u8 __pad[2];
+};
+
+struct prestera_rif_entry {
+	struct prestera_rif_entry_key {
+		struct prestera_iface iface;
+	} key;
+	struct prestera_vr *vr;
+	unsigned char addr[ETH_ALEN];
+	u16 hw_id; /* rif_id */
+	struct list_head router_node; /* ht */
+};
+
+struct prestera_rif_entry *
+prestera_rif_entry_find(const struct prestera_switch *sw,
+			const struct prestera_rif_entry_key *k);
+void prestera_rif_entry_destroy(struct prestera_switch *sw,
+				struct prestera_rif_entry *e);
+struct prestera_rif_entry *
+prestera_rif_entry_create(struct prestera_switch *sw,
+			  struct prestera_rif_entry_key *k,
+			  u32 tb_id, const unsigned char *addr);
+int prestera_router_hw_init(struct prestera_switch *sw);
+
+#endif /* _PRESTERA_ROUTER_HW_H_ */
-- 
2.17.1

Re: [PATCH net-next v2 4/6] net: marvell: prestera: add hardware router objects accounting

[Ido Schimmel]

On Mon, Dec 27, 2021 at 11:52:29PM +0200, Yevhen Orlov wrote:
> Add prestera_router_hw.c. This file contains functions, which track HW
> objects relations and links. This include implicity creation of objects,
> that needed by requested one and implicity removing of objects, which
> reference counter is became zero.
> 
> We need this layer, because kernel callbacks not always mapped to
> creation of single HW object. So let it be two different layers - one
> for subscribing and parsing kernel structures, and another
> (prestera_router_hw.c) for HW objects relations tracking.
> 
> There is two types of objects on router_hw layer:
>  - Explicit objects (rif_entry) : created by higher layer.
>  - Implicit objects (vr) : created on demand by explicit objects.
> 
> Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
> ---
> v1-->v2
> * No changes
> ---
>  .../net/ethernet/marvell/prestera/Makefile    |   2 +-
>  .../marvell/prestera/prestera_router.c        |  10 +
>  .../marvell/prestera/prestera_router_hw.c     | 209 ++++++++++++++++++
>  .../marvell/prestera/prestera_router_hw.h     |  36 +++
>  4 files changed, 256 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router_hw.c
>  create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_router_hw.h
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/Makefile b/drivers/net/ethernet/marvell/prestera/Makefile
> index ec69fc564a9f..d395f4131648 100644
> --- a/drivers/net/ethernet/marvell/prestera/Makefile
> +++ b/drivers/net/ethernet/marvell/prestera/Makefile
> @@ -4,6 +4,6 @@ prestera-objs		:= prestera_main.o prestera_hw.o prestera_dsa.o \
>  			   prestera_rxtx.o prestera_devlink.o prestera_ethtool.o \
>  			   prestera_switchdev.o prestera_acl.o prestera_flow.o \
>  			   prestera_flower.o prestera_span.o prestera_counter.o \
> -			   prestera_router.o
> +			   prestera_router.o prestera_router_hw.o
>  
>  obj-$(CONFIG_PRESTERA_PCI)	+= prestera_pci.o
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> index f3980d10eb29..2a32831df40f 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -5,10 +5,12 @@
>  #include <linux/types.h>
>  
>  #include "prestera.h"
> +#include "prestera_router_hw.h"
>  
>  int prestera_router_init(struct prestera_switch *sw)
>  {
>  	struct prestera_router *router;
> +	int err;
>  
>  	router = kzalloc(sizeof(*sw->router), GFP_KERNEL);
>  	if (!router)
> @@ -17,7 +19,15 @@ int prestera_router_init(struct prestera_switch *sw)
>  	sw->router = router;
>  	router->sw = sw;
>  
> +	err = prestera_router_hw_init(sw);
> +	if (err)
> +		goto err_router_lib_init;
> +
>  	return 0;
> +
> +err_router_lib_init:
> +	kfree(sw->router);
> +	return err;
>  }
>  
>  void prestera_router_fini(struct prestera_switch *sw)

Looks suspicious that you don't call prestera_router_hw_fini() here. You
can at least verify that the two lists you initialize in
prestera_router_hw_init() are indeed empty.

> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.c
> new file mode 100644
> index 000000000000..4f66fb21a299
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> +/* Copyright (c) 2019-2021 Marvell International Ltd. All rights reserved */
> +
> +#include <linux/rhashtable.h>
> +
> +#include "prestera.h"
> +#include "prestera_hw.h"
> +#include "prestera_router_hw.h"
> +#include "prestera_acl.h"
> +
> +/*            +--+
> + *   +------->|vr|
> + *   |        +--+
> + *   |
> + * +-+-------+
> + * |rif_entry|
> + * +---------+
> + *  Rif is
> + *  used as
> + *  entry point
> + *  for vr in hw
> + */
> +
> +int prestera_router_hw_init(struct prestera_switch *sw)
> +{
> +	INIT_LIST_HEAD(&sw->router->vr_list);
> +	INIT_LIST_HEAD(&sw->router->rif_entry_list);
> +
> +	return 0;
> +}
> +
> +static struct prestera_vr *__prestera_vr_find(struct prestera_switch *sw,
> +					      u32 tb_id)
> +{
> +	struct prestera_vr *vr;
> +
> +	list_for_each_entry(vr, &sw->router->vr_list, router_node) {

Probably better to store VRs in something like IDR instead of a linked
list

> +		if (vr->tb_id == tb_id)
> +			return vr;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct prestera_vr *__prestera_vr_create(struct prestera_switch *sw,
> +						u32 tb_id,
> +						struct netlink_ext_ack *extack)
> +{
> +	struct prestera_vr *vr;
> +	u16 hw_vr_id;
> +	int err;
> +
> +	err = prestera_hw_vr_create(sw, &hw_vr_id);
> +	if (err)
> +		return ERR_PTR(-ENOMEM);
> +
> +	vr = kzalloc(sizeof(*vr), GFP_KERNEL);
> +	if (!vr) {
> +		err = -ENOMEM;
> +		goto err_alloc_vr;
> +	}
> +
> +	vr->tb_id = tb_id;
> +	vr->hw_vr_id = hw_vr_id;
> +
> +	list_add(&vr->router_node, &sw->router->vr_list);
> +
> +	return vr;
> +
> +err_alloc_vr:
> +	prestera_hw_vr_delete(sw, hw_vr_id);
> +	kfree(vr);

You failed to allocate it, so no need to free it

> +	return ERR_PTR(err);
> +}
> +
> +static void __prestera_vr_destroy(struct prestera_switch *sw,
> +				  struct prestera_vr *vr)
> +{
> +	prestera_hw_vr_delete(sw, vr->hw_vr_id);
> +	list_del(&vr->router_node);

Not symmetric with __prestera_vr_create()

> +	kfree(vr);
> +}
> +
> +static struct prestera_vr *prestera_vr_get(struct prestera_switch *sw, u32 tb_id,
> +					   struct netlink_ext_ack *extack)
> +{
> +	struct prestera_vr *vr;
> +
> +	vr = __prestera_vr_find(sw, tb_id);
> +	if (!vr)
> +		vr = __prestera_vr_create(sw, tb_id, extack);
> +	if (IS_ERR(vr))
> +		return ERR_CAST(vr);
> +
> +	return vr;
> +}
> +
> +static void prestera_vr_put(struct prestera_switch *sw, struct prestera_vr *vr)
> +{
> +	if (!vr->ref_cnt)
> +		__prestera_vr_destroy(sw, vr);
> +}

These two functions should increase/decrease the reference count of the
VR

> +
> +/* iface is overhead struct. vr_id also can be removed. */

Unclear

> +static int
> +__prestera_rif_entry_key_copy(const struct prestera_rif_entry_key *in,
> +			      struct prestera_rif_entry_key *out)
> +{
> +	memset(out, 0, sizeof(*out));
> +
> +	switch (in->iface.type) {
> +	case PRESTERA_IF_PORT_E:
> +		out->iface.dev_port.hw_dev_num = in->iface.dev_port.hw_dev_num;
> +		out->iface.dev_port.port_num = in->iface.dev_port.port_num;
> +		break;
> +	case PRESTERA_IF_LAG_E:
> +		out->iface.lag_id = in->iface.lag_id;
> +		break;
> +	case PRESTERA_IF_VID_E:
> +		out->iface.vlan_id = in->iface.vlan_id;
> +		break;
> +	default:
> +		pr_err("Unsupported iface type");

If this should never happen, then consider using WARN_ON(1)

> +		return -EINVAL;
> +	}
> +
> +	out->iface.type = in->iface.type;
> +	return 0;
> +}
> +
> +struct prestera_rif_entry *
> +prestera_rif_entry_find(const struct prestera_switch *sw,
> +			const struct prestera_rif_entry_key *k)
> +{
> +	struct prestera_rif_entry *rif_entry;
> +	struct prestera_rif_entry_key lk; /* lookup key */
> +
> +	if (__prestera_rif_entry_key_copy(k, &lk))
> +		return NULL;
> +
> +	list_for_each_entry(rif_entry, &sw->router->rif_entry_list,
> +			    router_node) {
> +		if (!memcmp(k, &rif_entry->key, sizeof(*k)))
> +			return rif_entry;

Looks like rhashtable is a better option than a linked list

> +	}
> +
> +	return NULL;
> +}
> +
> +void prestera_rif_entry_destroy(struct prestera_switch *sw,
> +				struct prestera_rif_entry *e)

It's easier to maintain/review code that follows a pattern of create()
followed by destroy(). You can see if the error path is the same as what
you have in destroy()

> +{
> +	struct prestera_iface iface;
> +
> +	list_del(&e->router_node);
> +
> +	memcpy(&iface, &e->key.iface, sizeof(iface));
> +	iface.vr_id = e->vr->hw_vr_id;
> +	prestera_hw_rif_delete(sw, e->hw_id, &iface);
> +
> +	e->vr->ref_cnt--;
> +	prestera_vr_put(sw, e->vr);
> +	kfree(e);
> +}
> +
> +struct prestera_rif_entry *
> +prestera_rif_entry_create(struct prestera_switch *sw,
> +			  struct prestera_rif_entry_key *k,
> +			  u32 tb_id, const unsigned char *addr)
> +{
> +	int err;
> +	struct prestera_rif_entry *e;
> +	struct prestera_iface iface;
> +
> +	e = kzalloc(sizeof(*e), GFP_KERNEL);
> +	if (!e)
> +		goto err_kzalloc;
> +
> +	if (__prestera_rif_entry_key_copy(k, &e->key))
> +		goto err_key_copy;
> +
> +	e->vr = prestera_vr_get(sw, tb_id, NULL);
> +	if (IS_ERR(e->vr))
> +		goto err_vr_get;
> +
> +	e->vr->ref_cnt++;
> +	memcpy(&e->addr, addr, sizeof(e->addr));
> +
> +	/* HW */
> +	memcpy(&iface, &e->key.iface, sizeof(iface));
> +	iface.vr_id = e->vr->hw_vr_id;
> +	err = prestera_hw_rif_create(sw, &iface, e->addr, &e->hw_id);
> +	if (err)
> +		goto err_hw_create;
> +
> +	list_add(&e->router_node, &sw->router->rif_entry_list);
> +
> +	return e;
> +
> +err_hw_create:
> +	e->vr->ref_cnt--;
> +	prestera_vr_put(sw, e->vr);
> +err_vr_get:
> +err_key_copy:
> +	kfree(e);
> +err_kzalloc:
> +	return NULL;
> +}
> +
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.h
> new file mode 100644
> index 000000000000..fed53595f7bb
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router_hw.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
> +/* Copyright (c) 2019-2021 Marvell International Ltd. All rights reserved. */
> +
> +#ifndef _PRESTERA_ROUTER_HW_H_
> +#define _PRESTERA_ROUTER_HW_H_
> +
> +struct prestera_vr {
> +	struct list_head router_node;
> +	unsigned int ref_cnt;

Use refcount_t

> +	u32 tb_id;			/* key (kernel fib table id) */
> +	u16 hw_vr_id;			/* virtual router ID */
> +	u8 __pad[2];
> +};
> +
> +struct prestera_rif_entry {
> +	struct prestera_rif_entry_key {
> +		struct prestera_iface iface;
> +	} key;
> +	struct prestera_vr *vr;
> +	unsigned char addr[ETH_ALEN];
> +	u16 hw_id; /* rif_id */
> +	struct list_head router_node; /* ht */
> +};
> +
> +struct prestera_rif_entry *
> +prestera_rif_entry_find(const struct prestera_switch *sw,
> +			const struct prestera_rif_entry_key *k);
> +void prestera_rif_entry_destroy(struct prestera_switch *sw,
> +				struct prestera_rif_entry *e);
> +struct prestera_rif_entry *
> +prestera_rif_entry_create(struct prestera_switch *sw,
> +			  struct prestera_rif_entry_key *k,
> +			  u32 tb_id, const unsigned char *addr);
> +int prestera_router_hw_init(struct prestera_switch *sw);
> +
> +#endif /* _PRESTERA_ROUTER_HW_H_ */
> -- 
> 2.17.1
> 

[PATCH net-next v2 5/6] net: marvell: prestera: Register inetaddr stub notifiers

[Yevhen Orlov]

Initial implementation of notification handlers. For now this is just
stub.
So that we can move forward and add prestera_router_hw's objects
manipulations.

We support several addresses on interface. We just have nothing to do for
second address, because rif is already enabled on this interface, after
first one.

Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
---
v1-->v2
* Update commit message: explanation about addresses on rif
---
 .../net/ethernet/marvell/prestera/prestera.h  |   4 +
 .../ethernet/marvell/prestera/prestera_main.c |   2 +-
 .../marvell/prestera/prestera_router.c        | 105 ++++++++++++++++++
 3 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
index 7160da678457..a0a5a8e6bd8c 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera.h
@@ -281,6 +281,8 @@ struct prestera_router {
 	struct prestera_switch *sw;
 	struct list_head vr_list;
 	struct list_head rif_entry_list;
+	struct notifier_block inetaddr_nb;
+	struct notifier_block inetaddr_valid_nb;
 	bool aborted;
 };
 
@@ -328,6 +330,8 @@ int prestera_port_pvid_set(struct prestera_port *port, u16 vid);
 
 bool prestera_netdev_check(const struct net_device *dev);
 
+int prestera_is_valid_mac_addr(struct prestera_port *port, const u8 *addr);
+
 bool prestera_port_is_lag_member(const struct prestera_port *port);
 
 struct prestera_lag *prestera_lag_by_id(struct prestera_switch *sw, u16 id);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index 242904fcd866..5e45a4cda8cc 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -159,7 +159,7 @@ static netdev_tx_t prestera_port_xmit(struct sk_buff *skb,
 	return prestera_rxtx_xmit(netdev_priv(dev), skb);
 }
 
-static int prestera_is_valid_mac_addr(struct prestera_port *port, u8 *addr)
+int prestera_is_valid_mac_addr(struct prestera_port *port, const u8 *addr)
 {
 	if (!is_valid_ether_addr(addr))
 		return -EADDRNOTAVAIL;
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
index 2a32831df40f..0eb5f5e00e4e 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
@@ -3,10 +3,98 @@
 
 #include <linux/kernel.h>
 #include <linux/types.h>
+#include <linux/inetdevice.h>
 
 #include "prestera.h"
 #include "prestera_router_hw.h"
 
+static int __prestera_inetaddr_port_event(struct net_device *port_dev,
+					  unsigned long event,
+					  struct netlink_ext_ack *extack)
+{
+	struct prestera_port *port = netdev_priv(port_dev);
+	int err;
+
+	err = prestera_is_valid_mac_addr(port, port_dev->dev_addr);
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack, "RIF MAC must have the same prefix");
+		return err;
+	}
+
+	switch (event) {
+	case NETDEV_UP:
+	case NETDEV_DOWN:
+		break;
+	}
+
+	return 0;
+}
+
+static int __prestera_inetaddr_event(struct prestera_switch *sw,
+				     struct net_device *dev,
+				     unsigned long event,
+				     struct netlink_ext_ack *extack)
+{
+	if (prestera_netdev_check(dev) && !netif_is_bridge_port(dev) &&
+	    !netif_is_lag_port(dev) && !netif_is_ovs_port(dev))
+		return __prestera_inetaddr_port_event(dev, event, extack);
+
+	return 0;
+}
+
+static int __prestera_inetaddr_cb(struct notifier_block *nb,
+				  unsigned long event, void *ptr)
+{
+	struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
+	struct net_device *dev = ifa->ifa_dev->dev;
+	struct prestera_router *router = container_of(nb,
+						      struct prestera_router,
+						      inetaddr_nb);
+	struct in_device *idev;
+	int err = 0;
+
+	if (event != NETDEV_DOWN)
+		goto out;
+
+	/* Ignore if this is not latest address */
+	idev = __in_dev_get_rtnl(dev);
+	if (idev && idev->ifa_list)
+		goto out;
+
+	err = __prestera_inetaddr_event(router->sw, dev, event, NULL);
+out:
+	return notifier_from_errno(err);
+}
+
+static int __prestera_inetaddr_valid_cb(struct notifier_block *nb,
+					unsigned long event, void *ptr)
+{
+	struct in_validator_info *ivi = (struct in_validator_info *)ptr;
+	struct net_device *dev = ivi->ivi_dev->dev;
+	struct prestera_router *router = container_of(nb,
+						      struct prestera_router,
+						      inetaddr_valid_nb);
+	struct in_device *idev;
+	int err = 0;
+
+	if (event != NETDEV_UP)
+		goto out;
+
+	/* Ignore if this is not first address */
+	idev = __in_dev_get_rtnl(dev);
+	if (idev && idev->ifa_list)
+		goto out;
+
+	if (ipv4_is_multicast(ivi->ivi_addr)) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	err = __prestera_inetaddr_event(router->sw, dev, event, ivi->extack);
+out:
+	return notifier_from_errno(err);
+}
+
 int prestera_router_init(struct prestera_switch *sw)
 {
 	struct prestera_router *router;
@@ -23,8 +111,22 @@ int prestera_router_init(struct prestera_switch *sw)
 	if (err)
 		goto err_router_lib_init;
 
+	router->inetaddr_valid_nb.notifier_call = __prestera_inetaddr_valid_cb;
+	err = register_inetaddr_validator_notifier(&router->inetaddr_valid_nb);
+	if (err)
+		goto err_register_inetaddr_validator_notifier;
+
+	router->inetaddr_nb.notifier_call = __prestera_inetaddr_cb;
+	err = register_inetaddr_notifier(&router->inetaddr_nb);
+	if (err)
+		goto err_register_inetaddr_notifier;
+
 	return 0;
 
+err_register_inetaddr_notifier:
+	unregister_inetaddr_validator_notifier(&router->inetaddr_valid_nb);
+err_register_inetaddr_validator_notifier:
+	/* prestera_router_hw_fini */
 err_router_lib_init:
 	kfree(sw->router);
 	return err;
@@ -32,6 +134,9 @@ int prestera_router_init(struct prestera_switch *sw)
 
 void prestera_router_fini(struct prestera_switch *sw)
 {
+	unregister_inetaddr_notifier(&sw->router->inetaddr_nb);
+	unregister_inetaddr_validator_notifier(&sw->router->inetaddr_valid_nb);
+	/* router_hw_fini */
 	kfree(sw->router);
 	sw->router = NULL;
 }
-- 
2.17.1

Re: [PATCH net-next v2 5/6] net: marvell: prestera: Register inetaddr stub notifiers

[Ido Schimmel]

On Mon, Dec 27, 2021 at 11:52:30PM +0200, Yevhen Orlov wrote:
> Initial implementation of notification handlers. For now this is just
> stub.
> So that we can move forward and add prestera_router_hw's objects
> manipulations.
> 
> We support several addresses on interface. We just have nothing to do for
> second address, because rif is already enabled on this interface, after
> first one.
> 
> Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
> ---
> v1-->v2
> * Update commit message: explanation about addresses on rif
> ---
>  .../net/ethernet/marvell/prestera/prestera.h  |   4 +
>  .../ethernet/marvell/prestera/prestera_main.c |   2 +-
>  .../marvell/prestera/prestera_router.c        | 105 ++++++++++++++++++
>  3 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
> index 7160da678457..a0a5a8e6bd8c 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera.h
> @@ -281,6 +281,8 @@ struct prestera_router {
>  	struct prestera_switch *sw;
>  	struct list_head vr_list;
>  	struct list_head rif_entry_list;
> +	struct notifier_block inetaddr_nb;
> +	struct notifier_block inetaddr_valid_nb;
>  	bool aborted;
>  };
>  
> @@ -328,6 +330,8 @@ int prestera_port_pvid_set(struct prestera_port *port, u16 vid);
>  
>  bool prestera_netdev_check(const struct net_device *dev);
>  
> +int prestera_is_valid_mac_addr(struct prestera_port *port, const u8 *addr);
> +
>  bool prestera_port_is_lag_member(const struct prestera_port *port);
>  
>  struct prestera_lag *prestera_lag_by_id(struct prestera_switch *sw, u16 id);
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> index 242904fcd866..5e45a4cda8cc 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> @@ -159,7 +159,7 @@ static netdev_tx_t prestera_port_xmit(struct sk_buff *skb,
>  	return prestera_rxtx_xmit(netdev_priv(dev), skb);
>  }
>  
> -static int prestera_is_valid_mac_addr(struct prestera_port *port, u8 *addr)
> +int prestera_is_valid_mac_addr(struct prestera_port *port, const u8 *addr)
>  {
>  	if (!is_valid_ether_addr(addr))
>  		return -EADDRNOTAVAIL;
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> index 2a32831df40f..0eb5f5e00e4e 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -3,10 +3,98 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/types.h>
> +#include <linux/inetdevice.h>
>  
>  #include "prestera.h"
>  #include "prestera_router_hw.h"
>  
> +static int __prestera_inetaddr_port_event(struct net_device *port_dev,
> +					  unsigned long event,
> +					  struct netlink_ext_ack *extack)
> +{
> +	struct prestera_port *port = netdev_priv(port_dev);
> +	int err;
> +
> +	err = prestera_is_valid_mac_addr(port, port_dev->dev_addr);
> +	if (err) {
> +		NL_SET_ERR_MSG_MOD(extack, "RIF MAC must have the same prefix");
> +		return err;
> +	}
> +
> +	switch (event) {
> +	case NETDEV_UP:
> +	case NETDEV_DOWN:
> +		break;
> +	}

If you are only implementing these in the next patch, then add these
then

> +
> +	return 0;
> +}
> +
> +static int __prestera_inetaddr_event(struct prestera_switch *sw,
> +				     struct net_device *dev,
> +				     unsigned long event,
> +				     struct netlink_ext_ack *extack)
> +{
> +	if (prestera_netdev_check(dev) && !netif_is_bridge_port(dev) &&
> +	    !netif_is_lag_port(dev) && !netif_is_ovs_port(dev))

Your netdev notifier doesn't allow linking to an OVS bridge, so I'm not
sure what is the purpose of this check

Also, better use early return

What happens to that RIF when the port is linked to a bridge or unlinked
from one?

> +		return __prestera_inetaddr_port_event(dev, event, extack);
> +
> +	return 0;
> +}
> +
> +static int __prestera_inetaddr_cb(struct notifier_block *nb,
> +				  unsigned long event, void *ptr)
> +{
> +	struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
> +	struct net_device *dev = ifa->ifa_dev->dev;
> +	struct prestera_router *router = container_of(nb,
> +						      struct prestera_router,
> +						      inetaddr_nb);
> +	struct in_device *idev;
> +	int err = 0;
> +
> +	if (event != NETDEV_DOWN)
> +		goto out;
> +
> +	/* Ignore if this is not latest address */
> +	idev = __in_dev_get_rtnl(dev);
> +	if (idev && idev->ifa_list)
> +		goto out;
> +
> +	err = __prestera_inetaddr_event(router->sw, dev, event, NULL);
> +out:
> +	return notifier_from_errno(err);
> +}
> +
> +static int __prestera_inetaddr_valid_cb(struct notifier_block *nb,
> +					unsigned long event, void *ptr)
> +{
> +	struct in_validator_info *ivi = (struct in_validator_info *)ptr;
> +	struct net_device *dev = ivi->ivi_dev->dev;
> +	struct prestera_router *router = container_of(nb,
> +						      struct prestera_router,
> +						      inetaddr_valid_nb);
> +	struct in_device *idev;
> +	int err = 0;
> +
> +	if (event != NETDEV_UP)
> +		goto out;
> +
> +	/* Ignore if this is not first address */
> +	idev = __in_dev_get_rtnl(dev);
> +	if (idev && idev->ifa_list)
> +		goto out;
> +
> +	if (ipv4_is_multicast(ivi->ivi_addr)) {
> +		err = -EINVAL;

Use extack

> +		goto out;
> +	}
> +
> +	err = __prestera_inetaddr_event(router->sw, dev, event, ivi->extack);
> +out:
> +	return notifier_from_errno(err);
> +}
> +
>  int prestera_router_init(struct prestera_switch *sw)
>  {
>  	struct prestera_router *router;
> @@ -23,8 +111,22 @@ int prestera_router_init(struct prestera_switch *sw)
>  	if (err)
>  		goto err_router_lib_init;
>  
> +	router->inetaddr_valid_nb.notifier_call = __prestera_inetaddr_valid_cb;
> +	err = register_inetaddr_validator_notifier(&router->inetaddr_valid_nb);
> +	if (err)
> +		goto err_register_inetaddr_validator_notifier;
> +
> +	router->inetaddr_nb.notifier_call = __prestera_inetaddr_cb;
> +	err = register_inetaddr_notifier(&router->inetaddr_nb);
> +	if (err)
> +		goto err_register_inetaddr_notifier;
> +
>  	return 0;
>  
> +err_register_inetaddr_notifier:
> +	unregister_inetaddr_validator_notifier(&router->inetaddr_valid_nb);
> +err_register_inetaddr_validator_notifier:
> +	/* prestera_router_hw_fini */

Just create this function

>  err_router_lib_init:
>  	kfree(sw->router);
>  	return err;
> @@ -32,6 +134,9 @@ int prestera_router_init(struct prestera_switch *sw)
>  
>  void prestera_router_fini(struct prestera_switch *sw)
>  {
> +	unregister_inetaddr_notifier(&sw->router->inetaddr_nb);
> +	unregister_inetaddr_validator_notifier(&sw->router->inetaddr_valid_nb);
> +	/* router_hw_fini */

Likewise

>  	kfree(sw->router);
>  	sw->router = NULL;
>  }
> -- 
> 2.17.1
> 

Re: [PATCH net-next v2 5/6] net: marvell: prestera: Register inetaddr stub notifiers

[Yevhen Orlov]

On Thu, Dec 30, 2021 at 04:34:42PM +0200, Ido Schimmel wrote:
>
> What happens to that RIF when the port is linked to a bridge or unlinked
> from one?
>

We doesn't support any "RIF with bridge" scenario for now.
This restriction mentioned in cover latter.

Re: [PATCH net-next v2 5/6] net: marvell: prestera: Register inetaddr stub notifiers

[Andrew Lunn]

On Fri, Jan 07, 2022 at 03:42:56AM +0200, Yevhen Orlov wrote:
> On Thu, Dec 30, 2021 at 04:34:42PM +0200, Ido Schimmel wrote:
> >
> > What happens to that RIF when the port is linked to a bridge or unlinked
> > from one?
> >
> 
> We doesn't support any "RIF with bridge" scenario for now.
> This restriction mentioned in cover latter.

I did not look at the code. Does it return -EOPNOTSUPP? And then
bridging is performed by the host in software?

	 Andrew

     

Re: [PATCH net-next v2 5/6] net: marvell: prestera: Register inetaddr stub notifiers

[Yevhen Orlov]

On Fri, Jan 07, 2022 at 02:43:03PM +0100, Andrew Lunn wrote:
> On Fri, Jan 07, 2022 at 03:42:56AM +0200, Yevhen Orlov wrote:
> > On Thu, Dec 30, 2021 at 04:34:42PM +0200, Ido Schimmel wrote:
> > >
> > > What happens to that RIF when the port is linked to a bridge or unlinked
> > > from one?
> > >
> > 
> > We doesn't support any "RIF with bridge" scenario for now.
> > This restriction mentioned in cover latter.
> 
> I did not look at the code. Does it return -EOPNOTSUPP? And then
> bridging is performed by the host in software?
> 

Perhaps this is what you looking for:
> static int __prestera_inetaddr_event(struct prestera_switch *sw,
>                                 ┆    struct net_device *dev,
>                                 ┆    unsigned long event,
>                                 ┆    struct netlink_ext_ack *extack)
> {
>         if (!prestera_netdev_check(dev) || netif_is_bridge_port(dev) ||
>             netif_is_lag_port(dev) || netif_is_ovs_port(dev))
>                 return 0;

We just do nothing for bridge port.
Note: we cannot return ENOTSUPP here, because this will make
impossible to add address on bridge interface at all.

[PATCH net-next v2 6/6] net: marvell: prestera: Implement initial inetaddr notifiers

[Yevhen Orlov]

Add inetaddr notifiers to support add/del IPv4 address on switchdev
port. We create TRAP on first address, added on port and delete TRAP,
when last address removed.
Currently, driver supports only regular port to became routed.
Other port type support will be added later

Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
---
v1-->v2
* Remove useless assigment in prestera_fix_tb_id
---
 .../marvell/prestera/prestera_router.c        | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
index 0eb5f5e00e4e..483f0ba45ce0 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
@@ -4,16 +4,31 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/inetdevice.h>
+#include <net/switchdev.h>
 
 #include "prestera.h"
 #include "prestera_router_hw.h"
 
+/* This util to be used, to convert kernel rules for default vr in hw_vr */
+static u32 prestera_fix_tb_id(u32 tb_id)
+{
+	if (tb_id == RT_TABLE_UNSPEC ||
+	    tb_id == RT_TABLE_LOCAL ||
+	    tb_id == RT_TABLE_DEFAULT)
+		tb_id = RT_TABLE_MAIN;
+
+	return tb_id;
+}
+
 static int __prestera_inetaddr_port_event(struct net_device *port_dev,
 					  unsigned long event,
 					  struct netlink_ext_ack *extack)
 {
 	struct prestera_port *port = netdev_priv(port_dev);
 	int err;
+	struct prestera_rif_entry *re;
+	struct prestera_rif_entry_key re_key = {};
+	u32 kern_tb_id;
 
 	err = prestera_is_valid_mac_addr(port, port_dev->dev_addr);
 	if (err) {
@@ -21,9 +36,34 @@ static int __prestera_inetaddr_port_event(struct net_device *port_dev,
 		return err;
 	}
 
+	kern_tb_id = l3mdev_fib_table(port_dev);
+	re_key.iface.type = PRESTERA_IF_PORT_E;
+	re_key.iface.dev_port.hw_dev_num  = port->dev_id;
+	re_key.iface.dev_port.port_num  = port->hw_id;
+	re = prestera_rif_entry_find(port->sw, &re_key);
+
 	switch (event) {
 	case NETDEV_UP:
+		if (re) {
+			NL_SET_ERR_MSG_MOD(extack, "rif_entry already exist");
+			return -EEXIST;
+		}
+		re = prestera_rif_entry_create(port->sw, &re_key,
+					       prestera_fix_tb_id(kern_tb_id),
+					       port_dev->dev_addr);
+		if (!re) {
+			NL_SET_ERR_MSG_MOD(extack, "Can't create rif_entry");
+			return -EINVAL;
+		}
+		dev_hold(port_dev);
+		break;
 	case NETDEV_DOWN:
+		if (!re) {
+			NL_SET_ERR_MSG_MOD(extack, "rif_entry not exist");
+			return -EEXIST;
+		}
+		prestera_rif_entry_destroy(port->sw, re);
+		dev_put(port_dev);
 		break;
 	}
 
-- 
2.17.1

Re: [PATCH net-next v2 6/6] net: marvell: prestera: Implement initial inetaddr notifiers

[Ido Schimmel]

On Mon, Dec 27, 2021 at 11:52:31PM +0200, Yevhen Orlov wrote:
> Add inetaddr notifiers to support add/del IPv4 address on switchdev
> port. We create TRAP on first address, added on port and delete TRAP,
> when last address removed.
> Currently, driver supports only regular port to became routed.
> Other port type support will be added later
> 
> Co-developed-by: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
> ---
> v1-->v2
> * Remove useless assigment in prestera_fix_tb_id
> ---
>  .../marvell/prestera/prestera_router.c        | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> index 0eb5f5e00e4e..483f0ba45ce0 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -4,16 +4,31 @@
>  #include <linux/kernel.h>
>  #include <linux/types.h>
>  #include <linux/inetdevice.h>
> +#include <net/switchdev.h>
>  
>  #include "prestera.h"
>  #include "prestera_router_hw.h"
>  
> +/* This util to be used, to convert kernel rules for default vr in hw_vr */
> +static u32 prestera_fix_tb_id(u32 tb_id)
> +{
> +	if (tb_id == RT_TABLE_UNSPEC ||
> +	    tb_id == RT_TABLE_LOCAL ||
> +	    tb_id == RT_TABLE_DEFAULT)
> +		tb_id = RT_TABLE_MAIN;
> +
> +	return tb_id;
> +}
> +
>  static int __prestera_inetaddr_port_event(struct net_device *port_dev,
>  					  unsigned long event,
>  					  struct netlink_ext_ack *extack)
>  {
>  	struct prestera_port *port = netdev_priv(port_dev);
>  	int err;
> +	struct prestera_rif_entry *re;
> +	struct prestera_rif_entry_key re_key = {};
> +	u32 kern_tb_id;

Reverse xmas tree

>  
>  	err = prestera_is_valid_mac_addr(port, port_dev->dev_addr);
>  	if (err) {
> @@ -21,9 +36,34 @@ static int __prestera_inetaddr_port_event(struct net_device *port_dev,
>  		return err;
>  	}
>  
> +	kern_tb_id = l3mdev_fib_table(port_dev);
> +	re_key.iface.type = PRESTERA_IF_PORT_E;
> +	re_key.iface.dev_port.hw_dev_num  = port->dev_id;
> +	re_key.iface.dev_port.port_num  = port->hw_id;
> +	re = prestera_rif_entry_find(port->sw, &re_key);
> +
>  	switch (event) {
>  	case NETDEV_UP:
> +		if (re) {
> +			NL_SET_ERR_MSG_MOD(extack, "rif_entry already exist");

These messages are communicated to user space so use a message that is
more user friendly / informative

> +			return -EEXIST;
> +		}
> +		re = prestera_rif_entry_create(port->sw, &re_key,
> +					       prestera_fix_tb_id(kern_tb_id),
> +					       port_dev->dev_addr);
> +		if (!re) {
> +			NL_SET_ERR_MSG_MOD(extack, "Can't create rif_entry");
> +			return -EINVAL;
> +		}
> +		dev_hold(port_dev);

What is the purpose of this dev_hold()?

> +		break;
>  	case NETDEV_DOWN:
> +		if (!re) {
> +			NL_SET_ERR_MSG_MOD(extack, "rif_entry not exist");
> +			return -EEXIST;
> +		}
> +		prestera_rif_entry_destroy(port->sw, re);
> +		dev_put(port_dev);
>  		break;
>  	}
>  
> -- 
> 2.17.1
> 

Re: [PATCH net-next v2 6/6] net: marvell: prestera: Implement initial inetaddr notifiers

[Yevhen Orlov]

On Thu, Dec 30, 2021 at 04:39:23PM +0200, Ido Schimmel wrote:
> > +			return -EEXIST;
> > +		}
> > +		re = prestera_rif_entry_create(port->sw, &re_key,
> > +					       prestera_fix_tb_id(kern_tb_id),
> > +					       port_dev->dev_addr);
> > +		if (!re) {
> > +			NL_SET_ERR_MSG_MOD(extack, "Can't create rif_entry");
> > +			return -EINVAL;
> > +		}
> > +		dev_hold(port_dev);
>
> What is the purpose of this dev_hold()?
>

To make sure the port_dev is not freed before rif destroyed.