diff mbox series

[ovs-dev] odp-util: Add missing comma in format_odp_conntrack_action()

Message ID 166635852137.444103.15469799188583645502.stgit@fed.void
State Changes Requested
Headers show
Series [ovs-dev] odp-util: Add missing comma in format_odp_conntrack_action() | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Paolo Valerio Oct. 21, 2022, 1:22 p.m. UTC
If OVS_CT_ATTR_TIMEOUT is included, the resulting output is
the following:

actions:ct(commit,timeout=1nat(src=10.1.1.240))

Fix it by trivially adding a trailing ',' to timeout as well.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
 lib/odp-util.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ilya Maximets Oct. 25, 2022, 9:45 p.m. UTC | #1
On 10/21/22 15:22, Paolo Valerio wrote:
> If OVS_CT_ATTR_TIMEOUT is included, the resulting output is
> the following:
> 
> actions:ct(commit,timeout=1nat(src=10.1.1.240))
> 
> Fix it by trivially adding a trailing ',' to timeout as well.
> 
> Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
> ---
>  lib/odp-util.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index ba5be4bb3..72e076e1c 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -1004,7 +1004,7 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr)
>              ds_put_format(ds, "helper=%s,", helper);
>          }
>          if (timeout) {
> -            ds_put_format(ds, "timeout=%s", timeout);
> +            ds_put_format(ds, "timeout=%s,", timeout);
>          }
>          if (nat) {
>              format_odp_ct_nat(ds, nat);
> 

Hi.  Thanks for the patch!
Could you also, please, add a test case to tests/odp.at for this?

Best regards, Ilya Maximets.
Paolo Valerio Oct. 26, 2022, 8:49 a.m. UTC | #2
Ilya Maximets <i.maximets@ovn.org> writes:

> On 10/21/22 15:22, Paolo Valerio wrote:
>> If OVS_CT_ATTR_TIMEOUT is included, the resulting output is
>> the following:
>> 
>> actions:ct(commit,timeout=1nat(src=10.1.1.240))
>> 
>> Fix it by trivially adding a trailing ',' to timeout as well.
>> 
>> Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
>> ---
>>  lib/odp-util.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/lib/odp-util.c b/lib/odp-util.c
>> index ba5be4bb3..72e076e1c 100644
>> --- a/lib/odp-util.c
>> +++ b/lib/odp-util.c
>> @@ -1004,7 +1004,7 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr)
>>              ds_put_format(ds, "helper=%s,", helper);
>>          }
>>          if (timeout) {
>> -            ds_put_format(ds, "timeout=%s", timeout);
>> +            ds_put_format(ds, "timeout=%s,", timeout);
>>          }
>>          if (nat) {
>>              format_odp_ct_nat(ds, nat);
>> 
>
> Hi.  Thanks for the patch!
> Could you also, please, add a test case to tests/odp.at for this?

Sure, thanks for pointing that out.
Sent v2:

https://patchwork.ozlabs.org/project/openvswitch/patch/166677384931.806968.5359905777279608036.stgit@fed.void/

>
> Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/odp-util.c b/lib/odp-util.c
index ba5be4bb3..72e076e1c 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1004,7 +1004,7 @@  format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr)
             ds_put_format(ds, "helper=%s,", helper);
         }
         if (timeout) {
-            ds_put_format(ds, "timeout=%s", timeout);
+            ds_put_format(ds, "timeout=%s,", timeout);
         }
         if (nat) {
             format_odp_ct_nat(ds, nat);