[ovs-dev] [PATCH ovn] ovn: Remove SBREC_TABLE_HA_CHASSIS ctl_table

2022-10-24 Thread Jun Gu 
From: "jun.gu" 

Fix bug that ovn-sbctl daemon process exit when list non-existed
ha_chassis.

Submitted-at: https://github.com/ovn-org/ovn/pull/151
Signed-off-by: Jun Gu 
---
 utilities/ovn-sbctl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index e35556d34a..f60dde1b67 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -1431,9 +1431,6 @@ static const struct ctl_table_class 
tables[SBREC_N_TABLES] = {
 [SBREC_TABLE_HA_CHASSIS_GROUP].row_ids[0]
 = {_ha_chassis_group_col_name, NULL, NULL},
 
-[SBREC_TABLE_HA_CHASSIS].row_ids[0]
-= {_ha_chassis_col_chassis, NULL, NULL},
-
 [SBREC_TABLE_METER].row_ids[0]
 = {_meter_col_name, NULL, NULL},
 
-- 
2.31.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH net-next v4] net: openvswitch: Check vport netdev name

2024-04-19 Thread Jun Gu 
Ensure that the provided netdev name is not one of its aliases to
prevent unnecessary creation and destruction of the vport by
ovs-vswitchd.

Signed-off-by: Jun Gu 
Acked-by: Eelco Chaudron 
---
 net/openvswitch/vport-netdev.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 903537a5da22..618edc346c0f 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,7 +78,10 @@ struct vport *ovs_netdev_link(struct vport *vport, const 
char *name)
int err;
 
vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-   if (!vport->dev) {
+   /* Ensure that the device exists and that the provided
+* name is not one of its aliases.
+*/
+   if (!vport->dev || strcmp(name, ovs_vport_name(vport))) {
err = -ENODEV;
goto error_free_vport;
}
-- 
2.25.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH net-next v4] net: openvswitch: Check vport netdev name

2024-04-18 Thread Jun Gu 
From: "jun.gu" 

Ensure that the provided netdev name is not one of its aliases to
prevent unnecessary creation and destruction of the vport by
ovs-vswitchd.

Signed-off-by: jun.gu  
Acked-by: Eelco Chaudron 
---
 net/openvswitch/vport-netdev.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 903537a5da22..618edc346c0f 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,7 +78,10 @@ struct vport *ovs_netdev_link(struct vport *vport, const 
char *name)
int err;
 
vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-   if (!vport->dev) {
+   /* Ensure that the device exists and that the provided
+* name is not one of its aliases.
+*/
+   if (!vport->dev || strcmp(name, ovs_vport_name(vport))) {
err = -ENODEV;
goto error_free_vport;
}
-- 
2.25.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v4] net: openvswitch: Check vport netdev name

2024-04-22 Thread Jun Gu 
My mistake, I'm wondering that I need to submit a new patch or a v5 
version based on this patch?


在 4/23/24 08:38, Jakub Kicinski 写道:

On Fri, 19 Apr 2024 14:14:25 +0800 Jun Gu wrote:

vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-   if (!vport->dev) {
+   /* Ensure that the device exists and that the provided
+* name is not one of its aliases.
+*/
+   if (!vport->dev || strcmp(name, ovs_vport_name(vport))) {
err = -ENODEV;
goto error_free_vport;
}


Sorry I applied this before I realised that it's buggy.
dev_get_by_name() will give you a reference on the device.
You must free it, so the error handling is different.
Please follow up ASAP to fix that.


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] net: openvswitch: Check vport name

2024-04-16 Thread Jun Gu 

Hi Chaudron, thanks for your reply. Considerthe following scenario:

- set a net device alias name (such as *enpxx*) into ovs.

- |OVS_VPORT_CMD_NEW -> ||dev_get_by_name can query the net device using 
*enpxx* name, but the dev->name that return is *ensxx* name. the |net_device|struct including: ``` |struct net_device {

charname[IFNAMSIZ];
RH_KABI_REPLACE(struct hlist_node name_hlist,
struct netdev_name_node *name_node)
...
|``` name_hlist include enpxx and ensxx name and both of them point to 
the same net_device, the net_device's name is ensxx. So when using 
*enpxx* name to query net device, the ||net device's name that return is 
*ensxx* name.|

Then, openvswitch.ko will save the net device that name is*ensxx*  to a hash 
table. So this will cause the below process:
- ovs can'tobtain the enpxx net device by |OVS_VPORT_CMD_GET.|
-|dpif_netlink_port_poll will get a |notification after |OVS_VPORT_CMD_NEW|, 
but the vport's name is ensxx. ovs will query the ensxx net device from 
interface table, but nothing. So ovs will run the port_del operation.
- this will cause|OVS_VPORT_CMD_NEW|  and OVS_VPORT_CMD_DEL operation to run 
repeatedly.
So the above issue can be avoided by the patch.

在 4/15/24 18:04, Eelco Chaudron 写道:


On 13 Apr 2024, at 10:48, jun.gu wrote:


Check vport name from dev_get_by_name, this can avoid to add and remove
NIC repeatedly when NIC rename failed at system startup.

Signed-off-by: Jun Gu
---
  net/openvswitch/vport-netdev.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 903537a5da22..de8977d7f329 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,7 +78,7 @@ struct vport *ovs_netdev_link(struct vport *vport, const char 
*name)
int err;

vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-   if (!vport->dev) {
+   if (!vport->dev) || strcmp(name, ovs_vport_name(vport)) {

Hi Jun, not sure if I get the point here, as ovs_vport_name() translates into 
vport->dev->name.

So are we trying to catch the interface rename between the dev_get_by_name(), 
and the code below? This rename could happen at any other place, so this check 
does not guarantee anything. Or am I missing something?


err = -ENODEV;
goto error_free_vport;
}
--
2.25.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH net-next] net: openvswitch: Release reference to netdev

2024-04-23 Thread Jun Gu 
dev_get_by_name will provide a reference on the netdev. So ensure that
the reference of netdev is released after completed.

Fixes: 2540088b836f ("net: openvswitch: Check vport netdev name")
Signed-off-by: Jun Gu 
---
 net/openvswitch/vport-netdev.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 618edc346c0f..91a11067e458 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,12 +78,16 @@ struct vport *ovs_netdev_link(struct vport *vport, const 
char *name)
int err;
 
vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
+   if (!vport->dev) {
+   err = -ENODEV;
+   goto error_free_vport;
+   }
/* Ensure that the device exists and that the provided
 * name is not one of its aliases.
 */
-   if (!vport->dev || strcmp(name, ovs_vport_name(vport))) {
+   if (strcmp(name, ovs_vport_name(vport))) {
err = -ENODEV;
-   goto error_free_vport;
+   goto error_put;
}
netdev_tracker_alloc(vport->dev, >dev_tracker, GFP_KERNEL);
if (vport->dev->flags & IFF_LOOPBACK ||
-- 
2.25.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev