| Message ID | afce6563-313b-6c3f-3dd3-12ab0eea7ec0@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Jul 15, 2016 at 08:33:59PM +0530, Numan Siddique wrote: > To support 128-bits in ct_label, the value of the ct_label is expected > as a hex string in the 'ct_commit' action. > > Added a new accessor in the 'mf_subvalue' struct to access ovs_be128 > values. > > Signed-Off-by: Numan Siddique <nusiddiq@redhat.com> Only the "s" should be capitalized in "Signed-off-by". I know that's petty but Git only properly recognizes it with the right capitalization. > @@ -982,9 +982,8 @@ > <code>ct_mark</code> and/or <code>ct_label</code> will be set to the > values indicated by <var>value[/mask]</var> on the connection > tracking entry. <code>ct_mark</code> is a 32-bit field. > - <code>ct_label</code> is technically a 128-bit field, though OVN > - currently only supports 64-bits and will later be extended to > - support the full 128-bits. > + <code>ct_label</code> is a 128-bit field. The <var>value[/mask]</var> > + should be specified in hex string if > 64-bits are to be used. It seems asking for trouble to put a literal > into XML, so I rephrased this to avoid it. > </p> With that change, I applied this to master. Thank you!
On Mon, Jul 18, 2016 at 11:40 PM, Ben Pfaff <blp@ovn.org> wrote: > On Fri, Jul 15, 2016 at 08:33:59PM +0530, Numan Siddique wrote: > > To support 128-bits in ct_label, the value of the ct_label is expected > > as a hex string in the 'ct_commit' action. > > > > Added a new accessor in the 'mf_subvalue' struct to access ovs_be128 > > values. > > > > Signed-Off-by: Numan Siddique <nusiddiq@redhat.com> > > Only the "s" should be capitalized in "Signed-off-by". I know that's > petty but Git only properly recognizes it with the right capitalization. > I was doing the same mistake all this time. Thanks for pointing it out. I will take care of it from next time. > > @@ -982,9 +982,8 @@ > > <code>ct_mark</code> and/or <code>ct_label</code> will be > set to the > > values indicated by <var>value[/mask]</var> on the > connection > > tracking entry. <code>ct_mark</code> is a 32-bit field. > > - <code>ct_label</code> is technically a 128-bit field, > though OVN > > - currently only supports 64-bits and will later be extended > to > > - support the full 128-bits. > > + <code>ct_label</code> is a 128-bit field. The > <var>value[/mask]</var> > > + should be specified in hex string if > 64-bits are to be > used. > > It seems asking for trouble to put a literal > into XML, so I rephrased > this to avoid it. > > > </p> > > With that change, I applied this to master. Thank you! >
On Tue, Jul 19, 2016 at 1:24 AM, Numan Siddique <nusiddiq@redhat.com> wrote: > On Mon, Jul 18, 2016 at 11:40 PM, Ben Pfaff <blp@ovn.org> wrote: > > > On Fri, Jul 15, 2016 at 08:33:59PM +0530, Numan Siddique wrote: > > > To support 128-bits in ct_label, the value of the ct_label is expected > > > as a hex string in the 'ct_commit' action. > > > > > > Added a new accessor in the 'mf_subvalue' struct to access ovs_be128 > > > values. > > > > > > Signed-Off-by: Numan Siddique <nusiddiq@redhat.com> > > > > Only the "s" should be capitalized in "Signed-off-by". I know that's > > petty but Git only properly recognizes it with the right capitalization. > > > > I was doing the same mistake all this time. Thanks for pointing it out. I > will take care of it from next time. An easy way to do this is to use the "-s" option to "git commit". It automatically adds the header for you in the correct format.
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h index e2e9220..196868a 100644 --- a/include/openvswitch/meta-flow.h +++ b/include/openvswitch/meta-flow.h @@ -2006,6 +2006,10 @@ union mf_subvalue { ovs_be64 integer; }; struct { + ovs_be128 dummy_be128[7]; + ovs_be128 be128_int; + }; + struct { uint8_t dummy_mac[122]; struct eth_addr mac; }; diff --git a/ovn/TODO b/ovn/TODO index 4f134a4..0a6225d 100644 --- a/ovn/TODO +++ b/ovn/TODO @@ -38,12 +38,6 @@ ovn-sb.xml includes a tentative specification for this action. IPv6 will probably need an action or actions for ND that is similar to the "arp" action, and an action for generating -*** ct_label 128-bit support. - -We only support 64-bits for the ct_label argument to ct_commit(), but ct_label -is a 128-bit field. The OVN lexer only supports parsing 64-bit integers, but -we can use parse_int_string() to support larger integers. - ** IPv6 *** ND versus ARP diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 3d10d61..dda8959 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -879,16 +879,18 @@ parse_ct_commit_arg(struct action_context *ctx, action_error(ctx, "Expected '=' after argument to ct_commit"); return false; } + + /* ct_label is a 128-bit field. The lexer supports 128-bit + * integers if its a hex string. The ct_label value should be specified + * in hex string if > 64-bits are to be used */ if (ctx->lexer->token.type == LEX_T_INTEGER) { - label_value->be64.lo = ctx->lexer->token.value.integer; + label_value->be64.lo = ctx->lexer->token.value.be128_int.be64.lo; + label_value->be64.hi = ctx->lexer->token.value.be128_int.be64.hi; } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) { - /* XXX Technically, ct_label is a 128-bit field. The lexer - * only supports 64-bit integers, so that's all we support - * here. More work is needed to use parse_int_string() - * to support the full 128-bits. */ - label_value->be64.lo = ctx->lexer->token.value.integer; - label_mask->be64.hi = 0; - label_mask->be64.lo = ctx->lexer->token.mask.integer; + label_value->be64.lo = ctx->lexer->token.value.be128_int.be64.lo; + label_value->be64.hi = ctx->lexer->token.value.be128_int.be64.hi; + label_mask->be64.lo = ctx->lexer->token.mask.be128_int.be64.lo; + label_mask->be64.hi = ctx->lexer->token.mask.be128_int.be64.hi; } else { action_error(ctx, "Expected integer after 'ct_label='"); return false; diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 7b45bbb..fa24969 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -982,9 +982,8 @@ <code>ct_mark</code> and/or <code>ct_label</code> will be set to the values indicated by <var>value[/mask]</var> on the connection tracking entry. <code>ct_mark</code> is a 32-bit field. - <code>ct_label</code> is technically a 128-bit field, though OVN - currently only supports 64-bits and will later be extended to - support the full 128-bits. + <code>ct_label</code> is a 128-bit field. The <var>value[/mask]</var> + should be specified in hex string if > 64-bits are to be used. </p> <p> diff --git a/tests/ovn.at b/tests/ovn.at index 12de125..452c148 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -585,6 +585,11 @@ ct_commit(ct_mark=1); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_fie ct_commit(ct_mark=1/1); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1/0x1->ct_mark)), prereqs=ip ct_commit(ct_label=1); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1->ct_label)), prereqs=ip ct_commit(ct_label=1/1); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1/0x1->ct_label)), prereqs=ip +ct_commit(ct_label=0x01020304050607080910111213141516); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1020304050607080910111213141516->ct_label)), prereqs=ip +ct_commit(ct_label=0x181716151413121110090807060504030201); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x16151413121110090807060504030201->ct_label)), prereqs=ip +ct_commit(ct_label=0x01000000000000000000000000000000/0x01000000000000000000000000000000); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1000000000000000000000000000000/0x1000000000000000000000000000000->ct_label)), prereqs=ip +ct_commit(ct_label=18446744073709551615); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0xffffffffffffffff->ct_label)), prereqs=ip +ct_commit(ct_label=18446744073709551616); => Decimal constants must be less than 2**64. ct_commit(ct_mark=1, ct_label=2); => actions=ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1->ct_mark,set_field:0x2->ct_label)), prereqs=ip # dnat
To support 128-bits in ct_label, the value of the ct_label is expected as a hex string in the 'ct_commit' action. Added a new accessor in the 'mf_subvalue' struct to access ovs_be128 values. Signed-Off-by: Numan Siddique <nusiddiq@redhat.com> --- include/openvswitch/meta-flow.h | 4 ++++ ovn/TODO | 6 ------ ovn/lib/actions.c | 18 ++++++++++-------- ovn/ovn-sb.xml | 5 ++--- tests/ovn.at | 5 +++++ 5 files changed, 21 insertions(+), 17 deletions(-)