Message ID | 1443820578-9287-3-git-send-email-joestringer@nicira.com |
---|---|
State | Accepted |
Headers | show |
On Fri, Oct 02, 2015 at 02:16:09PM -0700, Joe Stringer wrote: > Previously, reg_load would only understand 64-bit values passed to it. > This patch extends the parsing to handle larger fields, if they are > specified in hexadecimal. Internally they are stored as a single action, > but they are converted into multiple 64-bit modifications when > re-serialised. > > Signed-off-by: Joe Stringer <joestringer@nicira.com> > --- > v4: Reduce change to allowing >64bit values to be specified in hex. Would you mind adding a test that exercises a >64 bit example, at least to demonstrate that it is parsed and formatted correctly? Acked-by: Ben Pfaff <blp@nicira.com>
On 5 October 2015 at 15:27, Ben Pfaff <blp@nicira.com> wrote: > On Fri, Oct 02, 2015 at 02:16:09PM -0700, Joe Stringer wrote: >> Previously, reg_load would only understand 64-bit values passed to it. >> This patch extends the parsing to handle larger fields, if they are >> specified in hexadecimal. Internally they are stored as a single action, >> but they are converted into multiple 64-bit modifications when >> re-serialised. >> >> Signed-off-by: Joe Stringer <joestringer@nicira.com> >> --- >> v4: Reduce change to allowing >64bit values to be specified in hex. > > Would you mind adding a test that exercises a >64 bit example, at least > to demonstrate that it is parsed and formatted correctly? Sure. I was using a test that is added later with the ct_label support, but IPv6 should be able to be handled in a similar manner. > Acked-by: Ben Pfaff <blp@nicira.com> Thanks!
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 6c9e1cb..0b22ce1 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -2571,10 +2571,9 @@ static char * OVS_WARN_UNUSED_RESULT parse_reg_load(char *arg, struct ofpbuf *ofpacts) { struct ofpact_set_field *sf = ofpact_put_reg_load(ofpacts); - const char *full_arg = arg; - uint64_t value = strtoull(arg, (char **) &arg, 0); struct mf_subfield dst; char *key, *value_str; + union mf_value value; char *error; error = set_field_split_str(arg, &key, &value_str, NULL); @@ -2587,16 +2586,29 @@ parse_reg_load(char *arg, struct ofpbuf *ofpacts) return error; } - if (dst.n_bits < 64 && (value >> dst.n_bits) != 0) { - return xasprintf("%s: value %"PRIu64" does not fit into %d bits", - full_arg, value, dst.n_bits); + if (parse_int_string(value_str, (uint8_t *)&value, dst.field->n_bytes, + &key)) { + return xasprintf("%s: cannot parse integer value", arg); + } + + if (!bitwise_is_all_zeros(&value, dst.field->n_bytes, dst.n_bits, + dst.field->n_bytes * 8 - dst.n_bits)) { + struct ds ds; + + ds_init(&ds); + mf_format(dst.field, &value, NULL, &ds); + error = xasprintf("%s: value %s does not fit into %d bits", + arg, ds_cstr(&ds), dst.n_bits); + ds_destroy(&ds); + return error; } sf->field = dst.field; memset(&sf->value, 0, sizeof sf->value); - bitwise_put(value, &sf->value, dst.field->n_bytes, dst.ofs, dst.n_bits); - bitwise_put(UINT64_MAX, &sf->mask, - dst.field->n_bytes, dst.ofs, dst.n_bits); + bitwise_copy(&value, dst.field->n_bytes, 0, &sf->value, + dst.field->n_bytes, dst.ofs, dst.n_bits); + bitwise_one(&sf->mask, dst.field->n_bytes, dst.ofs, dst.n_bits); + return NULL; }
Previously, reg_load would only understand 64-bit values passed to it. This patch extends the parsing to handle larger fields, if they are specified in hexadecimal. Internally they are stored as a single action, but they are converted into multiple 64-bit modifications when re-serialised. Signed-off-by: Joe Stringer <joestringer@nicira.com> --- v4: Reduce change to allowing >64bit values to be specified in hex. --- lib/ofp-actions.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-)