[ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect.

2024-04-03 Thread Yuhao zhou via dev
From: "zhouyuhao.philozhou" 

When mod a flow table's name with table's prefix name, there
will be no change. Because when check whether the new and old
name are the same, only compare the length of the new name.

Case:
  table 10: "good"
  There will be no change if mod the table's name with "g" "go" "goo".

Signed-off-by: zhouyuhao.philozhou 
---
 ofproto/ofproto.c |  4 +++-
 tests/ofproto.at  | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 122a06f30..bf7ed91b1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -9293,7 +9293,9 @@ oftable_set_name(struct oftable *table, const char *name, 
int level)
 if (level >= table->name_level) {
 if (name) {
 if (name[0]) {
-if (!table->name || strncmp(name, table->name, len)) {
+if (!table->name
+|| strncmp(name, table->name, len)
+|| len != strlen(table->name)) {
 free(table->name);
 table->name = xmemdup0(name, len);
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2889f81fb..09c57b292 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2523,6 +2523,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features 
br0 |grep '^  table'],
   table 253:
 ])
 
+# Make sure that the new name is old table's name prefix can also take effect.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 3 name:thr])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^  table'],
+  [0], [dnl
+  table 0 ("zero"):
+  table 1 ("one"): ditto
+  table 2: ditto
+  table 3 ("thr"): ditto
+  tables 4...252: ditto
+  table 253:
+])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-- 
2.39.3 (Apple Git-146)

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


Re: [ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect.

2024-03-15 Thread Ilya Maximets
On 3/16/24 01:44, Ilya Maximets wrote:
> On 3/8/24 09:32, Yuhao zhou via dev wrote:
>> From: "zhouyuhao.philozhou" 
>>
>> When mod a flow table's name with table's prefix name, there
>> will be no change. Because when check whether the new and old
>> name are the same, only compare the length of the new name.
>>
>> Case:
>>   table 10: "good"
>>   There will be no change if mod the table's name with "g" "go" "goo".
>>
>> Signed-off-by: zhouyuhao.philozhou 
>> ---

Also, please, add a version number to the patch subject while sending
new versions, e.g. [PATCH v3] ofproto: ...

And add a small description on what changed between versions here, after
the '---', but before the diff.

Best regards, Ilya Maximets.

>>  ofproto/ofproto.c |  4 +++-
>>  tests/ofproto.at  | 12 
>>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> Good catch!
> 
>>
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index 122a06f30..bf7ed91b1 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -9293,7 +9293,9 @@ oftable_set_name(struct oftable *table, const char 
>> *name, int level)
>>  if (level >= table->name_level) {
>>  if (name) {
>>  if (name[0]) {
>> -if (!table->name || strncmp(name, table->name, len)) {
>> +if (!table->name
>> +|| strncmp(name, table->name, len)
>> +|| len != strlen(table->name)) {
> 
> Maybe we can just use OFP_MAX_TABLE_NAME_LEN in the strncmp call?
> Will it produce the same result?
> 
> Also, there is an oftable_may_set_name() function just below, it
> will need the same change.
> 
>>  free(table->name);
>>  table->name = xmemdup0(name, len);
>>  }
>> diff --git a/tests/ofproto.at b/tests/ofproto.at
>> index 2889f81fb..09c57b292 100644
>> --- a/tests/ofproto.at
>> +++ b/tests/ofproto.at
>> @@ -2523,6 +2523,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features 
>> br0 |grep '^  table'],
>>table 253:
>>  ])
>>  
>> +# Make sure that the new name is old table's name prefix can also take 
>> effect.
> 
> s/is old table's name/equal to the old name's/
> 
> Best regards, Ilya Maximets.

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


Re: [ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect.

2024-03-15 Thread Ilya Maximets
On 3/8/24 09:32, Yuhao zhou via dev wrote:
> From: "zhouyuhao.philozhou" 
> 
> When mod a flow table's name with table's prefix name, there
> will be no change. Because when check whether the new and old
> name are the same, only compare the length of the new name.
> 
> Case:
>   table 10: "good"
>   There will be no change if mod the table's name with "g" "go" "goo".
> 
> Signed-off-by: zhouyuhao.philozhou 
> ---
>  ofproto/ofproto.c |  4 +++-
>  tests/ofproto.at  | 12 
>  2 files changed, 15 insertions(+), 1 deletion(-)

Good catch!

> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 122a06f30..bf7ed91b1 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -9293,7 +9293,9 @@ oftable_set_name(struct oftable *table, const char 
> *name, int level)
>  if (level >= table->name_level) {
>  if (name) {
>  if (name[0]) {
> -if (!table->name || strncmp(name, table->name, len)) {
> +if (!table->name
> +|| strncmp(name, table->name, len)
> +|| len != strlen(table->name)) {

Maybe we can just use OFP_MAX_TABLE_NAME_LEN in the strncmp call?
Will it produce the same result?

Also, there is an oftable_may_set_name() function just below, it
will need the same change.

>  free(table->name);
>  table->name = xmemdup0(name, len);
>  }
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 2889f81fb..09c57b292 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -2523,6 +2523,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features 
> br0 |grep '^  table'],
>table 253:
>  ])
>  
> +# Make sure that the new name is old table's name prefix can also take 
> effect.

s/is old table's name/equal to the old name's/

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect.

2024-03-08 Thread Yuhao zhou via dev
From: "zhouyuhao.philozhou" 

When mod a flow table's name with table's prefix name, there
will be no change. Because when check whether the new and old
name are the same, only compare the length of the new name.

Case:
  table 10: "good"
  There will be no change if mod the table's name with "g" "go" "goo".

Signed-off-by: zhouyuhao.philozhou 
---
 ofproto/ofproto.c |  4 +++-
 tests/ofproto.at  | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 122a06f30..bf7ed91b1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -9293,7 +9293,9 @@ oftable_set_name(struct oftable *table, const char *name, 
int level)
 if (level >= table->name_level) {
 if (name) {
 if (name[0]) {
-if (!table->name || strncmp(name, table->name, len)) {
+if (!table->name
+|| strncmp(name, table->name, len)
+|| len != strlen(table->name)) {
 free(table->name);
 table->name = xmemdup0(name, len);
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2889f81fb..09c57b292 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2523,6 +2523,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features 
br0 |grep '^  table'],
   table 253:
 ])
 
+# Make sure that the new name is old table's name prefix can also take effect.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 3 name:thr])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^  table'],
+  [0], [dnl
+  table 0 ("zero"):
+  table 1 ("one"): ditto
+  table 2: ditto
+  table 3 ("thr"): ditto
+  tables 4...252: ditto
+  table 253:
+])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-- 
2.39.3 (Apple Git-146)

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


[ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect.

2024-03-08 Thread Yuhao zhou via dev
From: "zhouyuhao.philozhou" 

When mod a flow table's name with table's prefix name, there
will be no change. Because when check whether the new and old
name are the same, only compare the length of the new name.

Case:
  table 10: "good"
  There will be no change if mod the table's name with "g" "go" "goo".

Signed-off-by: zhouyuhao.philozhou 
---
 ofproto/ofproto.c |  4 +++-
 tests/ofproto.at  | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 122a06f30..bf7ed91b1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -9293,7 +9293,9 @@ oftable_set_name(struct oftable *table, const char *name, 
int level)
 if (level >= table->name_level) {
 if (name) {
 if (name[0]) {
-if (!table->name || strncmp(name, table->name, len)) {
+if (!table->name
+|| strncmp(name, table->name, len)
+|| len != strlen(table->name)) {
 free(table->name);
 table->name = xmemdup0(name, len);
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2889f81fb..b68881a27 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2418,6 +2418,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features 
br0 |grep '^  table'],
   table 253:
 ])
 
+# Make sure that the new name is old table's name prefix can also take effect.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 3 name:thr])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^  table'],
+  [0], [dnl
+  table 0 ("zero"):
+  table 1 ("one"): ditto
+  table 2: ditto
+  table 3 ("thr"): ditto
+  tables 4...252: ditto
+  table 253:
+])
+
 # Set some table names via OVSDB.
 AT_CHECK(
   [ovs-vsctl \
-- 
2.39.3 (Apple Git-146)

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


Re: [ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect

2024-03-07 Thread 0-day Robot
Bleep bloop.  Greetings Yuhao zhou, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: The subject summary should end with a dot.
Subject: ofproto: Fix mod flow table name not to take effect
Lines checked: 61, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect

2024-03-07 Thread Yuhao zhou via dev
From: "zhouyuhao.philozhou" 

When mod a flow table's name with table's prefix name, there
will be no change. Because when check whether the new and old
name are the same, only compare the length of the new name.

Case:
  table 10: "good"
  There will be no change if mod the table's name with "g" "go" "goo".

Signed-off-by: zhouyuhao.philozhou 
---
 ofproto/ofproto.c |  4 +++-
 tests/ofproto.at  | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 122a06f30..bf7ed91b1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -9293,7 +9293,9 @@ oftable_set_name(struct oftable *table, const char *name, 
int level)
 if (level >= table->name_level) {
 if (name) {
 if (name[0]) {
-if (!table->name || strncmp(name, table->name, len)) {
+if (!table->name
+|| strncmp(name, table->name, len)
+|| len != strlen(table->name)) {
 free(table->name);
 table->name = xmemdup0(name, len);
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2889f81fb..b68881a27 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2418,6 +2418,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features 
br0 |grep '^  table'],
   table 253:
 ])
 
+# Make sure that the new name is old table's name prefix can also take effect.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 3 name:thr])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^  table'],
+  [0], [dnl
+  table 0 ("zero"):
+  table 1 ("one"): ditto
+  table 2: ditto
+  table 3 ("thr"): ditto
+  tables 4...252: ditto
+  table 253:
+])
+
 # Set some table names via OVSDB.
 AT_CHECK(
   [ovs-vsctl \
-- 
2.39.3 (Apple Git-146)

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


Re: [ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect. When mod a flow table's name with table's prefix name, there will be no change.

2024-03-07 Thread 0-day Robot
Bleep bloop.  Greetings Yuhao zhou, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: The subject, ': ', is over 70 characters, i.e., 132.
Subject: ofproto: Fix mod flow table name not to take effect. When mod a flow 
table's name with table's prefix name, there will be no change.
Lines checked: 60, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect. When mod a flow table's name with table's prefix name, there will be no change.

2024-03-07 Thread Yuhao zhou via dev
From: "zhouyuhao.philozhou" 

Because when check whether the new and old name are the same, only
compare the length of the new name.

Case:
  table 10: "good"
  There will be no change if mod the table's name with "g" "go" "goo".

Signed-off-by: zhouyuhao.philozhou 
---
 ofproto/ofproto.c |  4 +++-
 tests/ofproto.at  | 12 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 122a06f30..bf7ed91b1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -9293,7 +9293,9 @@ oftable_set_name(struct oftable *table, const char *name, 
int level)
 if (level >= table->name_level) {
 if (name) {
 if (name[0]) {
-if (!table->name || strncmp(name, table->name, len)) {
+if (!table->name
+|| strncmp(name, table->name, len)
+|| len != strlen(table->name)) {
 free(table->name);
 table->name = xmemdup0(name, len);
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2889f81fb..b68881a27 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2418,6 +2418,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features 
br0 |grep '^  table'],
   table 253:
 ])
 
+# Make sure that the new name is old table's name prefix can also take effect.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 3 name:thr])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^  table'],
+  [0], [dnl
+  table 0 ("zero"):
+  table 1 ("one"): ditto
+  table 2: ditto
+  table 3 ("thr"): ditto
+  tables 4...252: ditto
+  table 253:
+])
+
 # Set some table names via OVSDB.
 AT_CHECK(
   [ovs-vsctl \
-- 
2.39.3 (Apple Git-146)

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