Re: [ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect.
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.
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
Re: [ovs-dev] [PATCH] ofproto: Fix mod flow table name not to take effect
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
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.
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
