[ovs-dev] [PATCH ovn 1/2] controller: Use OpenFlow version 1.5

Numan Siddique numans at ovn.org
Tue May 5 18:00:54 UTC 2020


On Mon, May 4, 2020 at 9:22 AM Han Zhou <hzhou at ovn.org> wrote:

> On Thu, Apr 30, 2020 at 10:20 AM <numans at ovn.org> wrote:
> >
> > From: Numan Siddique <numans at ovn.org>
> >
> > When adding flows to the group table, we need to use OFP15_VERSION to
> > set the selection_method. Right now ovn-controller is setting
> > select_method=dp_hash for OVN load balancers, but when encoding the
> > group mod, it is ignored.
> >
> > Signed-off-by: Numan Siddique <numans at ovn.org>
> > ---
> >  controller/ofctrl.c         | 14 +++++++-------
> >  controller/ovn-controller.c |  2 +-
> >  controller/pinctrl.c        |  2 +-
> >  lib/actions.c               | 10 +++++-----
> >  lib/expr.c                  |  2 +-
> >  tests/ovn.at                |  6 +++---
> >  utilities/ovn-sbctl.c       |  4 ++--
> >  utilities/ovn-trace.c       |  4 ++--
> >  8 files changed, 22 insertions(+), 22 deletions(-)
> >
> > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > index 485a857d1..4b51cd86e 100644
> > --- a/controller/ofctrl.c
> > +++ b/controller/ofctrl.c
> > @@ -178,7 +178,7 @@ ofctrl_init(struct ovn_extend_table *group_table,
> >              int inactivity_probe_interval)
> >  {
> >      swconn = rconn_create(inactivity_probe_interval, 0,
> > -                          DSCP_DEFAULT, 1 << OFP13_VERSION);
> > +                          DSCP_DEFAULT, 1 << OFP15_VERSION);
> >      tx_counter = rconn_packet_counter_create();
> >      hmap_init(&installed_flows);
> >      ovs_list_init(&flow_updates);
> > @@ -282,8 +282,8 @@ process_tlv_table_reply(const struct
> ofputil_tlv_table_reply *reply)
> >      ovs_list_init(&ttm.mappings);
> >      ovs_list_push_back(&ttm.mappings, &tm.list_node);
> >
> > -    xid = queue_msg(ofputil_encode_tlv_table_mod(OFP13_VERSION, &ttm));
> > -    xid2 = queue_msg(ofputil_encode_barrier_request(OFP13_VERSION));
> > +    xid = queue_msg(ofputil_encode_tlv_table_mod(OFP15_VERSION, &ttm));
> > +    xid2 = queue_msg(ofputil_encode_barrier_request(OFP15_VERSION));
> >      state = S_TLV_TABLE_MOD_SENT;
> >
> >      return true;
> > @@ -911,7 +911,7 @@ encode_flow_mod(struct ofputil_flow_mod *fm)
> >      fm->buffer_id = UINT32_MAX;
> >      fm->out_port = OFPP_ANY;
> >      fm->out_group = OFPG_ANY;
> > -    return ofputil_encode_flow_mod(fm, OFPUTIL_P_OF13_OXM);
> > +    return ofputil_encode_flow_mod(fm, OFPUTIL_P_OF15_OXM);
> >  }
> >
> >  static void
> > @@ -926,7 +926,7 @@ add_flow_mod(struct ofputil_flow_mod *fm, struct
> ovs_list *msgs)
> >  static struct ofpbuf *
> >  encode_group_mod(const struct ofputil_group_mod *gm)
> >  {
> > -    return ofputil_encode_group_mod(OFP13_VERSION, gm, NULL, -1);
> > +    return ofputil_encode_group_mod(OFP15_VERSION, gm, NULL, -1);
> >  }
> >
> >  static void
> > @@ -940,7 +940,7 @@ add_group_mod(const struct ofputil_group_mod *gm,
> struct ovs_list *msgs)
> >  static struct ofpbuf *
> >  encode_meter_mod(const struct ofputil_meter_mod *mm)
> >  {
> > -    return ofputil_encode_meter_mod(OFP13_VERSION, mm);
> > +    return ofputil_encode_meter_mod(OFP15_VERSION, mm);
> >  }
> >
> >  static void
> > @@ -1281,7 +1281,7 @@ ofctrl_put(struct ovn_desired_flow_table
> *flow_table,
> >
> >      if (!ovs_list_is_empty(&msgs)) {
> >          /* Add a barrier to the list of messages. */
> > -        struct ofpbuf *barrier =
> ofputil_encode_barrier_request(OFP13_VERSION);
> > +        struct ofpbuf *barrier =
> ofputil_encode_barrier_request(OFP15_VERSION);
> >          const struct ofp_header *oh = barrier->data;
> >          ovs_be32 xid_ = oh->xid;
> >          ovs_list_push_back(&msgs, &barrier->list_node);
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 6ff897325..a2d92429c 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -2297,7 +2297,7 @@ parse_options(int argc, char *argv[])
> >              usage();
> >
> >          case 'V':
> > -            ovs_print_version(OFP13_VERSION, OFP13_VERSION);
> > +            ovs_print_version(OFP15_VERSION, OFP15_VERSION);
> >              exit(EXIT_SUCCESS);
> >
> >          VLOG_OPTION_HANDLERS
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index f0d63b9a6..4284c710b 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -2773,7 +2773,7 @@ pinctrl_handler(void *arg_)
> >      static long long int svc_monitors_next_run_time = LLONG_MAX;
> >      static long long int send_prefixd_time = LLONG_MAX;
> >
> > -    swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP13_VERSION);
> > +    swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
> >
> >      while (!latch_is_set(&pctrl->pinctrl_thread_exit)) {
> >          if (pctrl->br_int_name) {
> > diff --git a/lib/actions.c b/lib/actions.c
> > index 21c07969d..021a376b4 100644
> > --- a/lib/actions.c
> > +++ b/lib/actions.c
> > @@ -1508,7 +1508,7 @@ encode_nested_actions(const struct ovnact_nest *on,
> >      size_t oc_offset = encode_start_controller_op(opcode, false,
> >                                                    NX_CTLR_NO_METER,
> ofpacts);
> >      ofpacts_put_openflow_actions(inner_ofpacts.data, inner_ofpacts.size,
> > -                                 ofpacts, OFP13_VERSION);
> > +                                 ofpacts, OFP15_VERSION);
> >      encode_finish_controller_op(oc_offset, ofpacts);
> >
> >      /* Free memory. */
> > @@ -2311,7 +2311,7 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts
> *pdo,
> >      size_t oc_offset =
> encode_start_controller_op(ACTION_OPCODE_PUT_DHCP_OPTS,
> >                                                    true,
> NX_CTLR_NO_METER,
> >                                                    ofpacts);
> > -    nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
> > +    nx_put_header(ofpacts, dst.field->id, OFP15_VERSION, false);
> >      ovs_be32 ofs = htonl(dst.ofs);
> >      ofpbuf_put(ofpacts, &ofs, sizeof ofs);
> >
> > @@ -2342,7 +2342,7 @@ encode_PUT_DHCPV6_OPTS(const struct ovnact_put_opts
> *pdo,
> >
> >      size_t oc_offset = encode_start_controller_op(
> >          ACTION_OPCODE_PUT_DHCPV6_OPTS, true, NX_CTLR_NO_METER, ofpacts);
> > -    nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
> > +    nx_put_header(ofpacts, dst.field->id, OFP15_VERSION, false);
> >      ovs_be32 ofs = htonl(dst.ofs);
> >      ofpbuf_put(ofpacts, &ofs, sizeof ofs);
> >
> > @@ -2452,7 +2452,7 @@ encode_DNS_LOOKUP(const struct ovnact_dns_lookup
> *dl,
> >      size_t oc_offset =
> encode_start_controller_op(ACTION_OPCODE_DNS_LOOKUP,
> >                                                    true,
> NX_CTLR_NO_METER,
> >                                                    ofpacts);
> > -    nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
> > +    nx_put_header(ofpacts, dst.field->id, OFP15_VERSION, false);
> >      ovs_be32 ofs = htonl(dst.ofs);
> >      ofpbuf_put(ofpacts, &ofs, sizeof ofs);
> >      encode_finish_controller_op(oc_offset, ofpacts);
> > @@ -2616,7 +2616,7 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts
> *po,
> >
> >      size_t oc_offset = encode_start_controller_op(
> >          ACTION_OPCODE_PUT_ND_RA_OPTS, true, NX_CTLR_NO_METER, ofpacts);
> > -    nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
> > +    nx_put_header(ofpacts, dst.field->id, OFP15_VERSION, false);
> >      ovs_be32 ofs = htonl(dst.ofs);
> >      ofpbuf_put(ofpacts, &ofs, sizeof ofs);
> >
> > diff --git a/lib/expr.c b/lib/expr.c
> > index 78646a1af..078d17840 100644
> > --- a/lib/expr.c
> > +++ b/lib/expr.c
> > @@ -1414,7 +1414,7 @@ expr_symbol_format(const struct expr_symbol
> *symbol, struct ds *s)
> >      } else if (symbol->ovn_field) {
> >          ds_put_cstr(s, symbol->name);
> >      } else {
> > -        nx_format_field_name(symbol->field->id, OFP13_VERSION, s);
> > +        nx_format_field_name(symbol->field->id, OFP15_VERSION, s);
> >      }
> >  }
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index c04cd06d2..7befc8224 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -1215,7 +1215,7 @@ reg1[0] = put_dhcp_opts(offerip=1.2.3.4,
> domain_name=1.2.3.4);
> >
> >  # nd_ns
> >  nd_ns { nd.target = xxreg0; output; };
> > -    encodes as
>
> controller(userdata=00.00.00.09.00.00.00.00.ff.ff.00.18.00.00.23.20.00.06.00.80.00.00.00.00.00.01.de.10.00.01.2e.10.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> > +    encodes as
> controller(userdata=00.00.00.09.00.00.00.00.00.1c.00.18.00.
> 80.00.00.00.00.00.00.00.01.de
> .10.80.00.3e.10.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> >      has prereqs ip6
> >
> >  nd_ns { };
> > @@ -1226,12 +1226,12 @@ nd_ns { };
> >  # nd_na
> >  nd_na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc; outport
> = inport; inport = ""; /* Allow sending out inport. */ output; };
> >      formats as nd_na { eth.src = 12:34:56:78:9a:bc; nd.tll =
> 12:34:56:78:9a:bc; outport = inport; inport = ""; output; };
> > -    encodes as
>
> controller(userdata=00.00.00.03.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.00.19.00.10.00.01.1c.04.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> > +    encodes as
>
> controller(userdata=00.00.00.03.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.00.1c.00.18.00.20.00.00.00.00.00.00.00.01.1c.04.00.01.1e.04.00.00.00.00.00.19.00.10.00.01.1c.04.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> >      has prereqs nd_ns
> >  # nd_na_router
> >  nd_na_router { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc;
> outport = inport; inport = ""; /* Allow sending out inport. */ output; };
> >      formats as nd_na_router { eth.src = 12:34:56:78:9a:bc; nd.tll =
> 12:34:56:78:9a:bc; outport = inport; inport = ""; output; };
> > -    encodes as
>
> controller(userdata=00.00.00.0c.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.00.19.00.10.00.01.1c.04.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> > +    encodes as
>
> controller(userdata=00.00.00.0c.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.00.1c.00.18.00.20.00.00.00.00.00.00.00.01.1c.04.00.01.1e.04.00.00.00.00.00.19.00.10.00.01.1c.04.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> >      has prereqs nd_ns
> >
> >  # get_nd
> > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> > index d8bb3dcbc..04e082c70 100644
> > --- a/utilities/ovn-sbctl.c
> > +++ b/utilities/ovn-sbctl.c
> > @@ -795,7 +795,7 @@ sbctl_open_vconn(struct shash *options)
> >
> >      char *remote = ovs->data ? xstrdup(ovs->data) : default_ovs();
> >      struct vconn *vconn;
> > -    int retval = vconn_open_block(remote, 1 << OFP13_VERSION, 0, -1,
> &vconn);
> > +    int retval = vconn_open_block(remote, 1 << OFP15_VERSION, 0, -1,
> &vconn);
> >      if (retval) {
> >          VLOG_WARN("%s: connection failed (%s)", remote,
> ovs_strerror(retval));
> >      }
> > @@ -816,7 +816,7 @@ sbctl_dump_openflow(struct vconn *vconn, const struct
> uuid *uuid, bool stats)
> >
> >      struct ofputil_flow_stats *fses;
> >      size_t n_fses;
> > -    int error = vconn_dump_flows(vconn, &fsr, OFPUTIL_P_OF13_OXM,
> > +    int error = vconn_dump_flows(vconn, &fsr, OFPUTIL_P_OF15_OXM,
> >                                   &fses, &n_fses);
> >      if (error) {
> >          VLOG_WARN("%s: error obtaining flow stats (%s)",
> > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> > index c9d72285c..d7251e7ed 100644
> > --- a/utilities/ovn-trace.c
> > +++ b/utilities/ovn-trace.c
> > @@ -2326,7 +2326,7 @@ trace_openflow(const struct ovntrace_flow *f,
> struct ovs_list *super)
> >
> >      struct ofputil_flow_stats *fses;
> >      size_t n_fses;
> > -    int error = vconn_dump_flows(vconn, &fsr, OFPUTIL_P_OF13_OXM,
> > +    int error = vconn_dump_flows(vconn, &fsr, OFPUTIL_P_OF15_OXM,
> >                                   &fses, &n_fses);
> >      if (error) {
> >          ovntrace_node_append(super, OVNTRACE_NODE_ERROR,
> > @@ -2435,7 +2435,7 @@ trace(const char *dp_s, const char *flow_s)
> >      ds_put_char(&output, '\n');
> >
> >      if (ovs) {
> > -        int retval = vconn_open_block(ovs, 1 << OFP13_VERSION, 0, -1,
> &vconn);
> > +        int retval = vconn_open_block(ovs, 1 << OFP15_VERSION, 0, -1,
> &vconn);
> >          if (retval) {
> >              VLOG_WARN("%s: connection failed (%s)", ovs,
> ovs_strerror(retval));
> >          }
> > --
> > 2.25.4
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Acked-by: Han Zhou <hzhou at ovn.org>
>

Thanks. I applied this patch to master.

Numan


> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list