On Fri, Apr 19, 2024 at 6:38 PM Numan Siddique <[email protected]> wrote:

>
>
> On Fri, Apr 19, 2024 at 7:09 AM Ales Musil <[email protected]> wrote:
>
>> The current packet injection loses ct_state in the process. When
>> the ct_state is lost we might commit to DNAT zone and perform
>> zero SNAT after the packet injection. This causes the first session
>> to be wrong as the reply packets are not unDNATted.
>>
>> Instead of re-injecting the packet back into the pipeline when
>> we get the MAC binding, use paused controller action. The paused
>> controller action stores ct_state, which is important for the behavior
>> of the resumed packet.
>>
>> At the same time bump the OvS submodule latest branch-3.3. This is
>> mainly for [0], which fixes metering for paused controller actions.
>>
>> In order to make sure that the paused action works during upgrade add
>> the output implicitly. Once the upgrade is done northd will create option
>> to inform controllers that the implicit action is no longer needed.
>>
>> [0] c560f6ca3257 ("ofproto-dpif-xlate: Fix continuations with associated
>> metering.")
>>
>> Reported-at: https://issues.redhat.com/browse/FDP-439
>> Signed-off-by: Ales Musil <[email protected]>
>> ---
>> v5: Rebase on top of current main.
>> v4: Fix copy paste error in the global_handler.
>> v3: Rebase on top of current main.
>>     Add flag to ensure that the paused action works during upgrade.
>> v2: Fix the Jira link and add ack from Mark.
>>
>
> Thanks.  I applied this patch to the main.
> It doesn't apply cleanly to branch-24.03.  Please submit a backport patch
> for branches - 24.03 and 23.09.
>

Forgot to reply earlier, the backports are up on ML.


> Numan
>
>
> ---
>>  controller/lflow.c          |  1 +
>>  controller/lflow.h          |  1 +
>>  controller/mac-learn.c      | 30 ++++++++----
>>  controller/mac-learn.h      |  9 ++--
>>  controller/ovn-controller.c | 21 ++++++++
>>  controller/pinctrl.c        | 64 +++++++++----------------
>>  include/ovn/actions.h       |  3 ++
>>  lib/actions.c               | 47 +++++++++++++++++-
>>  northd/en-global-config.c   |  4 ++
>>  northd/northd.c             |  6 +--
>>  tests/multinode.at          |  8 ++++
>>  tests/ovn-northd.at         |  3 ++
>>  tests/ovn.at                |  8 ++--
>>  tests/system-ovn.at         | 95 +++++++++++++++++++++++++++++++++++++
>>  tests/test-ovn.c            |  1 +
>>  15 files changed, 239 insertions(+), 62 deletions(-)
>>
>> diff --git a/controller/lflow.c b/controller/lflow.c
>> index 895d17d19..760ec0b41 100644
>> --- a/controller/lflow.c
>> +++ b/controller/lflow.c
>> @@ -874,6 +874,7 @@ add_matches_to_flow_table(const struct
>> sbrec_logical_flow *lflow,
>>          .collector_ids = l_ctx_in->collector_ids,
>>          .lflow_uuid = lflow->header_.uuid,
>>          .dp_key = ldp->datapath->tunnel_key,
>> +        .explicit_arp_ns_output = l_ctx_in->explicit_arp_ns_output,
>>
>>          .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
>>          .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
>> diff --git a/controller/lflow.h b/controller/lflow.h
>> index 9b7ffa19c..295d004f4 100644
>> --- a/controller/lflow.h
>> +++ b/controller/lflow.h
>> @@ -130,6 +130,7 @@ struct lflow_ctx_in {
>>      bool lb_hairpin_use_ct_mark;
>>      bool localnet_learn_fdb;
>>      bool localnet_learn_fdb_changed;
>> +    bool explicit_arp_ns_output;
>>  };
>>
>>  struct lflow_ctx_out {
>> diff --git a/controller/mac-learn.c b/controller/mac-learn.c
>> index 071f01b4f..0c3b60c23 100644
>> --- a/controller/mac-learn.c
>> +++ b/controller/mac-learn.c
>> @@ -199,15 +199,24 @@ ovn_fdb_add(struct hmap *fdbs, uint32_t dp_key,
>> struct eth_addr mac,
>>  /* packet buffering functions */
>>
>>  struct packet_data *
>> -ovn_packet_data_create(struct ofpbuf ofpacts,
>> -                       const struct dp_packet *original_packet)
>> +ovn_packet_data_create(const struct ofputil_packet_in *pin,
>> +                       const struct ofpbuf *continuation)
>>  {
>>      struct packet_data *pd = xmalloc(sizeof *pd);
>>
>> -    pd->ofpacts = ofpacts;
>> -    /* clone the packet to send it later with correct L2 address */
>> -    pd->p = dp_packet_clone_data(dp_packet_data(original_packet),
>> -                                 dp_packet_size(original_packet));
>> +    pd->pin = (struct ofputil_packet_in) {
>> +        .packet = xmemdup(pin->packet, pin->packet_len),
>> +        .packet_len = pin->packet_len,
>> +        .flow_metadata = pin->flow_metadata,
>> +        .reason = pin->reason,
>> +        .table_id = pin->table_id,
>> +        .cookie = pin->cookie,
>> +        /* Userdata are empty on purpose,
>> +         * it is not needed for the continuation. */
>> +        .userdata = NULL,
>> +        .userdata_len = 0,
>> +    };
>> +    pd->continuation = ofpbuf_clone(continuation);
>>
>>      return pd;
>>  }
>> @@ -216,8 +225,8 @@ ovn_packet_data_create(struct ofpbuf ofpacts,
>>  void
>>  ovn_packet_data_destroy(struct packet_data *pd)
>>  {
>> -    dp_packet_delete(pd->p);
>> -    ofpbuf_uninit(&pd->ofpacts);
>> +    free(pd->pin.packet);
>> +    ofpbuf_delete(pd->continuation);
>>      free(pd);
>>  }
>>
>> @@ -307,7 +316,10 @@ ovn_buffered_packets_ctx_run(struct
>> buffered_packets_ctx *ctx,
>>
>>          struct packet_data *pd;
>>          LIST_FOR_EACH_POP (pd, node, &bp->queue) {
>> -            struct eth_header *eth = dp_packet_data(pd->p);
>> +            struct dp_packet packet;
>> +            dp_packet_use_const(&packet, pd->pin.packet,
>> pd->pin.packet_len);
>> +
>> +            struct eth_header *eth = dp_packet_data(&packet);
>>              eth->eth_dst = mac;
>>
>>              ovs_list_push_back(&ctx->ready_packets_data, &pd->node);
>> diff --git a/controller/mac-learn.h b/controller/mac-learn.h
>> index e0fd6a8d1..20a015e1a 100644
>> --- a/controller/mac-learn.h
>> +++ b/controller/mac-learn.h
>> @@ -24,6 +24,7 @@
>>  #include "openvswitch/hmap.h"
>>  #include "openvswitch/list.h"
>>  #include "openvswitch/ofpbuf.h"
>> +#include "openvswitch/ofp-packet.h"
>>
>>  struct ovsdb_idl_index;
>>
>> @@ -91,8 +92,8 @@ struct fdb_entry *ovn_fdb_add(struct hmap *fdbs,
>>  struct packet_data {
>>      struct ovs_list node;
>>
>> -    struct ofpbuf ofpacts;
>> -    struct dp_packet *p;
>> +    struct ofpbuf *continuation;
>> +    struct ofputil_packet_in pin;
>>  };
>>
>>  struct buffered_packets {
>> @@ -120,8 +121,8 @@ struct buffered_packets_ctx {
>>  };
>>
>>  struct packet_data *
>> -ovn_packet_data_create(struct ofpbuf ofpacts,
>> -                       const struct dp_packet *original_packet);
>> +ovn_packet_data_create(const struct ofputil_packet_in *pin,
>> +                       const struct ofpbuf *continuation);
>>  void ovn_packet_data_destroy(struct packet_data *pd);
>>  struct buffered_packets *
>>  ovn_buffered_packets_add(struct buffered_packets_ctx *ctx, uint64_t
>> dp_key,
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index b84f6dfd4..23269af83 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -3668,6 +3668,7 @@ non_vif_data_ovs_iface_handler(struct engine_node
>> *node, void *data OVS_UNUSED)
>>
>>  struct ed_type_northd_options {
>>      bool lb_hairpin_use_ct_mark;
>> +    bool explicit_arp_ns_output;
>>  };
>>
>>
>> @@ -3698,6 +3699,13 @@ en_northd_options_run(struct engine_node *node,
>> void *data)
>>          ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
>>                          DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
>>          : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
>> +
>> +    n_opts->explicit_arp_ns_output =
>> +            sb_global
>> +            ? smap_get_bool(&sb_global->options,
>> "arp_ns_explicit_output",
>> +                            false)
>> +            : false;
>> +
>>      engine_set_node_state(node, EN_UPDATED);
>>  }
>>
>> @@ -3720,6 +3728,18 @@ en_northd_options_sb_sb_global_handler(struct
>> engine_node *node, void *data)
>>          n_opts->lb_hairpin_use_ct_mark = lb_hairpin_use_ct_mark;
>>          engine_set_node_state(node, EN_UPDATED);
>>      }
>> +
>> +    bool explicit_arp_ns_output =
>> +            sb_global
>> +            ? smap_get_bool(&sb_global->options,
>> "arp_ns_explicit_output",
>> +                            false)
>> +            : false;
>> +
>> +    if (explicit_arp_ns_output != n_opts->explicit_arp_ns_output) {
>> +        n_opts->explicit_arp_ns_output = explicit_arp_ns_output;
>> +        engine_set_node_state(node, EN_UPDATED);
>> +    }
>> +
>>      return true;
>>  }
>>
>> @@ -3949,6 +3969,7 @@ init_lflow_ctx(struct engine_node *node,
>>      l_ctx_in->localnet_learn_fdb_changed =
>> rt_data->localnet_learn_fdb_changed;
>>      l_ctx_in->chassis_tunnels = &non_vif_data->chassis_tunnels;
>>      l_ctx_in->lb_hairpin_use_ct_mark = n_opts->lb_hairpin_use_ct_mark;
>> +    l_ctx_in->explicit_arp_ns_output = n_opts->explicit_arp_ns_output;
>>      l_ctx_in->nd_ra_opts = &fo->nd_ra_opts;
>>      l_ctx_in->dhcp_opts = &dhcp_opts->v4_opts;
>>      l_ctx_in->dhcpv6_opts = &dhcp_opts->v6_opts;
>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>> index 327ac0335..aa73facbf 100644
>> --- a/controller/pinctrl.c
>> +++ b/controller/pinctrl.c
>> @@ -258,9 +258,9 @@ static void pinctrl_handle_put_nd_ra_opts(
>>      struct ofpbuf *continuation);
>>  static void pinctrl_handle_nd_ns(struct rconn *swconn,
>>                                   const struct flow *ip_flow,
>> -                                 struct dp_packet *pkt_in,
>> -                                 const struct match *md,
>> -                                 struct ofpbuf *userdata);
>> +                                 const struct ofputil_packet_in *pin,
>> +                                 struct ofpbuf *userdata,
>> +                                 const struct ofpbuf *continuation);
>>  static void pinctrl_handle_put_icmp_frag_mtu(struct rconn *swconn,
>>                                               const struct flow *in_flow,
>>                                               struct dp_packet *pkt_in,
>> @@ -1534,11 +1534,13 @@ destroy_buffered_packets_ctx(void)
>>
>>  /* Called with in the pinctrl_handler thread context. */
>>  static void
>> -pinctrl_handle_buffered_packets(struct dp_packet *pkt_in,
>> -                                const struct match *md, bool is_arp)
>> +pinctrl_handle_buffered_packets(const struct ofputil_packet_in *pin,
>> +                                const struct ofpbuf *continuation,
>> +                                bool is_arp)
>>  OVS_REQUIRES(pinctrl_mutex)
>>  {
>>      struct in6_addr ip;
>> +    const struct match *md = &pin->flow_metadata;
>>      uint64_t dp_key = ntohll(md->flow.metadata);
>>      uint64_t oport_key = md->flow.regs[MFF_LOG_OUTPORT - MFF_REG0];
>>
>> @@ -1556,20 +1558,7 @@ OVS_REQUIRES(pinctrl_mutex)
>>          return;
>>      }
>>
>> -    struct ofpbuf ofpacts;
>> -    ofpbuf_init(&ofpacts, 4096);
>> -    reload_metadata(&ofpacts, md);
>> -    /* reload pkt_mark field */
>> -    const struct mf_field *pkt_mark_field = mf_from_id(MFF_PKT_MARK);
>> -    union mf_value pkt_mark_value;
>> -    mf_get_value(pkt_mark_field, &md->flow, &pkt_mark_value);
>> -    ofpact_put_set_field(&ofpacts, pkt_mark_field, &pkt_mark_value,
>> NULL);
>> -
>> -    struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts);
>> -    resubmit->in_port = OFPP_CONTROLLER;
>> -    resubmit->table_id = OFTABLE_OUTPUT_INIT;
>> -
>> -    struct packet_data *pd = ovn_packet_data_create(ofpacts, pkt_in);
>> +    struct packet_data *pd = ovn_packet_data_create(pin, continuation);
>>      ovn_buffered_packets_packet_data_enqueue(bp, pd);
>>
>>      /* There is a chance that the MAC binding was already created. */
>> @@ -1579,8 +1568,8 @@ OVS_REQUIRES(pinctrl_mutex)
>>  /* Called with in the pinctrl_handler thread context. */
>>  static void
>>  pinctrl_handle_arp(struct rconn *swconn, const struct flow *ip_flow,
>> -                   struct dp_packet *pkt_in,
>> -                   const struct match *md, struct ofpbuf *userdata)
>> +                   const struct ofputil_packet_in *pin,
>> +                   struct ofpbuf *userdata, const struct ofpbuf
>> *continuation)
>>  {
>>      /* This action only works for IP packets, and the switch should only
>> send
>>       * us IP packets this way, but check here just to be sure. */
>> @@ -1592,7 +1581,7 @@ pinctrl_handle_arp(struct rconn *swconn, const
>> struct flow *ip_flow,
>>      }
>>
>>      ovs_mutex_lock(&pinctrl_mutex);
>> -    pinctrl_handle_buffered_packets(pkt_in, md, true);
>> +    pinctrl_handle_buffered_packets(pin, continuation, true);
>>      ovs_mutex_unlock(&pinctrl_mutex);
>>
>>      /* Compose an ARP packet. */
>> @@ -1617,7 +1606,8 @@ pinctrl_handle_arp(struct rconn *swconn, const
>> struct flow *ip_flow,
>>                        ip_flow->vlans[0].tci);
>>      }
>>
>> -    set_actions_and_enqueue_msg(swconn, &packet, md, userdata);
>> +    set_actions_and_enqueue_msg(swconn, &packet,
>> +                                &pin->flow_metadata, userdata);
>>      dp_packet_uninit(&packet);
>>  }
>>
>> @@ -3294,8 +3284,7 @@ process_packet_in(struct rconn *swconn, const
>> struct ofp_header *msg)
>>
>>      switch (ntohl(ah->opcode)) {
>>      case ACTION_OPCODE_ARP:
>> -        pinctrl_handle_arp(swconn, &headers, &packet, &pin.flow_metadata,
>> -                           &userdata);
>> +        pinctrl_handle_arp(swconn, &headers, &pin, &userdata,
>> &continuation);
>>          break;
>>      case ACTION_OPCODE_IGMP:
>>          pinctrl_ip_mcast_handle(swconn, &headers, &packet,
>> &pin.flow_metadata,
>> @@ -3361,8 +3350,7 @@ process_packet_in(struct rconn *swconn, const
>> struct ofp_header *msg)
>>          break;
>>
>>      case ACTION_OPCODE_ND_NS:
>> -        pinctrl_handle_nd_ns(swconn, &headers, &packet,
>> &pin.flow_metadata,
>> -                             &userdata);
>> +        pinctrl_handle_nd_ns(swconn, &headers, &pin, &userdata,
>> &continuation);
>>          break;
>>
>>      case ACTION_OPCODE_ICMP:
>> @@ -4375,16 +4363,8 @@ send_mac_binding_buffered_pkts(struct rconn
>> *swconn)
>>
>>      struct packet_data *pd;
>>      LIST_FOR_EACH_POP (pd, node,
>> &buffered_packets_ctx.ready_packets_data) {
>> -        struct ofputil_packet_out po = {
>> -            .packet = dp_packet_data(pd->p),
>> -            .packet_len = dp_packet_size(pd->p),
>> -            .buffer_id = UINT32_MAX,
>> -            .ofpacts = pd->ofpacts.data,
>> -            .ofpacts_len = pd->ofpacts.size,
>> -        };
>> -        match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
>> -        queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
>> -
>> +        queue_msg(swconn, ofputil_encode_resume(&pd->pin,
>> pd->continuation,
>> +                                                proto));
>>          ovn_packet_data_destroy(pd);
>>      }
>>
>> @@ -6318,8 +6298,9 @@ pinctrl_handle_nd_na(struct rconn *swconn, const
>> struct flow *ip_flow,
>>  /* Called with in the pinctrl_handler thread context. */
>>  static void
>>  pinctrl_handle_nd_ns(struct rconn *swconn, const struct flow *ip_flow,
>> -                     struct dp_packet *pkt_in,
>> -                     const struct match *md, struct ofpbuf *userdata)
>> +                     const struct ofputil_packet_in *pin,
>> +                     struct ofpbuf *userdata,
>> +                     const struct ofpbuf *continuation)
>>  {
>>      /* This action only works for IPv6 packets. */
>>      if (get_dl_type(ip_flow) != htons(ETH_TYPE_IPV6)) {
>> @@ -6329,7 +6310,7 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const
>> struct flow *ip_flow,
>>      }
>>
>>      ovs_mutex_lock(&pinctrl_mutex);
>> -    pinctrl_handle_buffered_packets(pkt_in, md, false);
>> +    pinctrl_handle_buffered_packets(pin, continuation, false);
>>      ovs_mutex_unlock(&pinctrl_mutex);
>>
>>      uint64_t packet_stub[128 / 8];
>> @@ -6342,7 +6323,8 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const
>> struct flow *ip_flow,
>>                    &ip_flow->ipv6_dst);
>>
>>      /* Reload previous packet metadata and set actions from userdata. */
>> -    set_actions_and_enqueue_msg(swconn, &packet, md, userdata);
>> +    set_actions_and_enqueue_msg(swconn, &packet,
>> +                                &pin->flow_metadata, userdata);
>>      dp_packet_uninit(&packet);
>>  }
>>
>> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
>> index 8e794450c..f697dff39 100644
>> --- a/include/ovn/actions.h
>> +++ b/include/ovn/actions.h
>> @@ -854,6 +854,9 @@ struct ovnact_encode_params {
>>      /* The datapath key. */
>>      uint32_t dp_key;
>>
>> +    /* Indication if we should add explicit output after arp/nd_ns
>> action. */
>> +    bool explicit_arp_ns_output;
>> +
>>      /* OVN maps each logical flow table (ltable), one-to-one, onto a
>> physical
>>       * OpenFlow flow table (ptable).  A number of parameters describe
>> this
>>       * mapping and data related to flow tables:
>> diff --git a/lib/actions.c b/lib/actions.c
>> index 39bb5bc8a..361d55009 100644
>> --- a/lib/actions.c
>> +++ b/lib/actions.c
>> @@ -1836,6 +1836,44 @@ format_REJECT(const struct ovnact_nest *nest,
>> struct ds *s)
>>      format_nested_action(nest, "reject", s);
>>  }
>>
>> +static bool
>> +is_paused_nested_action(enum action_opcode opcode)
>> +{
>> +    switch (opcode) {
>> +    case ACTION_OPCODE_ARP:
>> +    case ACTION_OPCODE_ND_NS:
>> +        return true;
>> +    case ACTION_OPCODE_IGMP:
>> +    case ACTION_OPCODE_PUT_ARP:
>> +    case ACTION_OPCODE_PUT_DHCP_OPTS:
>> +    case ACTION_OPCODE_ND_NA:
>> +    case ACTION_OPCODE_ND_NA_ROUTER:
>> +    case ACTION_OPCODE_PUT_ND:
>> +    case ACTION_OPCODE_PUT_FDB:
>> +    case ACTION_OPCODE_PUT_DHCPV6_OPTS:
>> +    case ACTION_OPCODE_DNS_LOOKUP:
>> +    case ACTION_OPCODE_LOG:
>> +    case ACTION_OPCODE_PUT_ND_RA_OPTS:
>> +    case ACTION_OPCODE_ICMP:
>> +    case ACTION_OPCODE_ICMP4_ERROR:
>> +    case ACTION_OPCODE_ICMP6_ERROR:
>> +    case ACTION_OPCODE_TCP_RESET:
>> +    case ACTION_OPCODE_SCTP_ABORT:
>> +    case ACTION_OPCODE_REJECT:
>> +    case ACTION_OPCODE_PUT_ICMP4_FRAG_MTU:
>> +    case ACTION_OPCODE_PUT_ICMP6_FRAG_MTU:
>> +    case ACTION_OPCODE_EVENT:
>> +    case ACTION_OPCODE_BIND_VPORT:
>> +    case ACTION_OPCODE_DHCP6_SERVER:
>> +    case ACTION_OPCODE_HANDLE_SVC_CHECK:
>> +    case ACTION_OPCODE_BFD_MSG:
>> +    case ACTION_OPCODE_ACTIVATION_STRATEGY_RARP:
>> +    case ACTION_OPCODE_MG_SPLIT_BUF:
>> +    default:
>> +        return false;
>> +    }
>> +}
>> +
>>  static void
>>  encode_nested_actions(const struct ovnact_nest *on,
>>                        const struct ovnact_encode_params *ep,
>> @@ -1851,7 +1889,8 @@ encode_nested_actions(const struct ovnact_nest *on,
>>       * converted to OpenFlow, as its userdata.  ovn-controller will
>> convert the
>>       * packet to ARP or NA and then send the packet and actions back to
>> the
>>       * switch inside an OFPT_PACKET_OUT message. */
>> -    size_t oc_offset = encode_start_controller_op(opcode, false,
>> +    bool pause = is_paused_nested_action(opcode);
>> +    size_t oc_offset = encode_start_controller_op(opcode, pause,
>>                                                    ep->ctrl_meter_id,
>> ofpacts);
>>      ofpacts_put_openflow_actions(inner_ofpacts.data, inner_ofpacts.size,
>>                                   ofpacts, OFP15_VERSION);
>> @@ -1867,6 +1906,9 @@ encode_ARP(const struct ovnact_nest *on,
>>             struct ofpbuf *ofpacts)
>>  {
>>      encode_nested_actions(on, ep, ACTION_OPCODE_ARP, ofpacts);
>> +    if (!ep->explicit_arp_ns_output) {
>> +        emit_resubmit(ofpacts, ep->output_ptable);
>> +    }
>>  }
>>
>>  static void
>> @@ -1955,6 +1997,9 @@ encode_ND_NS(const struct ovnact_nest *on,
>>               struct ofpbuf *ofpacts)
>>  {
>>      encode_nested_actions(on, ep, ACTION_OPCODE_ND_NS, ofpacts);
>> +    if (!ep->explicit_arp_ns_output) {
>> +        emit_resubmit(ofpacts, ep->output_ptable);
>> +    }
>>  }
>>
>>  static void
>> diff --git a/northd/en-global-config.c b/northd/en-global-config.c
>> index 34e393b33..9462b3b7b 100644
>> --- a/northd/en-global-config.c
>> +++ b/northd/en-global-config.c
>> @@ -553,6 +553,10 @@ update_sb_config_options_to_sbrec(struct
>> ed_type_global_config *config_data,
>>          smap_replace(options, "sbctl_probe_interval", sip);
>>      }
>>
>> +    /* Adds indication that northd is handling explicit output after
>> +     * arp/nd_ns action. */
>> +    smap_add(options, "arp_ns_explicit_output", "true");
>> +
>>      if (!smap_equal(&sb->options, options)) {
>>          sbrec_sb_global_set_options(sb, options);
>>      }
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 37f443e70..331d9c267 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -13422,7 +13422,7 @@ build_arp_request_flows_for_lrouter(
>>                        "ip6.dst = %s; "
>>                        "nd.target = %s; "
>>                        "output; "
>> -                      "};", ETH_ADDR_ARGS(eth_dst), sn_addr_s,
>> +                      "}; output;", ETH_ADDR_ARGS(eth_dst), sn_addr_s,
>>                        route->nexthop);
>>
>>          ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_ARP_REQUEST,
>> 200,
>> @@ -13442,7 +13442,7 @@ build_arp_request_flows_for_lrouter(
>>                        "arp.tpa = " REG_NEXT_HOP_IPV4 "; "
>>                        "arp.op = 1; " /* ARP request */
>>                        "output; "
>> -                      "};",
>> +                      "}; output;",
>>                        copp_meter_get(COPP_ARP_RESOLVE, od->nbr->copp,
>>                                       meter_groups),
>>                        lflow_ref);
>> @@ -13451,7 +13451,7 @@ build_arp_request_flows_for_lrouter(
>>                        "nd_ns { "
>>                        "nd.target = " REG_NEXT_HOP_IPV6 "; "
>>                        "output; "
>> -                      "};",
>> +                      "}; output;",
>>                        copp_meter_get(COPP_ND_NS_RESOLVE, od->nbr->copp,
>>                                       meter_groups),
>>                        lflow_ref);
>> diff --git a/tests/multinode.at b/tests/multinode.at
>> index 0187382be..b959a2550 100644
>> --- a/tests/multinode.at
>> +++ b/tests/multinode.at
>> @@ -70,6 +70,14 @@ M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ping -q -c
>> 3 -i 0.3 -w 2 20.0.0.3 | F
>>  3 packets transmitted, 3 received, 0% packet loss, time 0ms
>>  ])
>>
>> +check multinode_nbctl lsp-set-addresses sw1-port1 unknown
>> +m_wait_for_ports_up sw1-port1
>> +
>> +M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.3 | FORMAT_PING], \
>> +[0], [dnl
>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> +])
>> +
>>  AT_CLEANUP
>>
>>  AT_SETUP([ovn multinode pmtu - distributed router - geneve])
>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>> index be006fb32..dcc29ffa8 100644
>> --- a/tests/ovn-northd.at
>> +++ b/tests/ovn-northd.at
>> @@ -6973,6 +6973,9 @@ ct_dnat /* assuming no un-dnat entry, so no change
>> */ {
>>              arp.op = 1;
>>              output("rtr-ls");
>>          };
>> +        ct_dnat /* assuming no un-dnat entry, so no change */ {
>> +            output("rtr-ls");
>> +        };
>>      };
>>  };
>>  ])
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index c8cc1d37f..dc6aafd53 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -1521,11 +1521,11 @@ clone { ip4.dst = 255.255.255.255; output; };
>> next;
>>
>>  # arp
>>  arp { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
>> -    encodes as
>> controller(userdata=00.00.00.00.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.OFTABLE_SAVE_INPORT_HEX.00.00.00),resubmit(,OFTABLE_SAVE_INPORT)
>> +    encodes as
>> controller(userdata=00.00.00.00.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.OFTABLE_SAVE_INPORT_HEX.00.00.00,pause),resubmit(,OFTABLE_SAVE_INPORT)
>>      has prereqs ip4
>>  arp { };
>>      formats as arp { drop; };
>> -    encodes as controller(userdata=00.00.00.00.00.00.00.00)
>> +    encodes as controller(userdata=00.00.00.00.00.00.00.00,pause)
>>      has prereqs ip4
>>
>>  # get_arp
>> @@ -1655,12 +1655,12 @@ reg1[[0]] = put_dhcp_opts(offerip=1.2.3.4,
>> domain_search_list=1.2.3.4);
>>
>>  # nd_ns
>>  nd_ns { nd.target = xxreg0; output; };
>> -    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.OFTABLE_SAVE_INPORT_HEX.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.OFTABLE_SAVE_INPORT_HEX.00.00.00,pause)
>>      has prereqs ip6
>>
>>  nd_ns { };
>>      formats as nd_ns { drop; };
>> -    encodes as controller(userdata=00.00.00.09.00.00.00.00)
>> +    encodes as controller(userdata=00.00.00.09.00.00.00.00,pause)
>>      has prereqs ip6
>>
>>  # nd_na
>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>> index 516fb4d99..5848f3901 100644
>> --- a/tests/system-ovn.at
>> +++ b/tests/system-ovn.at
>> @@ -12417,3 +12417,98 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query
>> port patch-.*/d
>>  /connection dropped.*/d"])
>>  AT_CLEANUP
>>  ])
>> +
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([LB with first packet buffered])
>> +AT_KEYWORDS([ovnlb])
>> +
>> +CHECK_CONNTRACK()
>> +CHECK_CONNTRACK_NAT()
>> +
>> +ovn_start
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +ADD_BR([br-int])
>> +ADD_BR([br-ext])
>> +
>> +check ovs-ofctl add-flow br-ext action=normal
>> +# Set external-ids in br-int needed for ovn-controller
>> +check ovs-vsctl \
>> +        -- set Open_vSwitch . external-ids:system-id=hv1 \
>> +        -- set Open_vSwitch .
>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>> +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>> +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>> +        -- set bridge br-int fail-mode=secure
>> other-config:disable-in-band=true \
>> +        -- set Open_vSwitch .
>> external-ids:ovn-bridge-mappings=phynet:br-ext
>> +
>> +# Start ovn-controller
>> +start_daemon ovn-controller
>> +
>> +check ovn-nbctl lr-add lr
>> +check ovn-nbctl ls-add internal
>> +check ovn-nbctl ls-add public
>> +
>> +check ovn-nbctl lrp-add lr lr-pub 00:00:01:01:02:03 192.168.100.1/24
>> 1000::1/64
>> +check ovn-nbctl lsp-add  public pub-lr -- set Logical_Switch_Port pub-lr
>> \
>> +    type=router options:router-port=lr-pub
>> addresses=\"00:00:01:01:02:03\"
>> +
>> +check ovn-nbctl lrp-add lr lr-internal 00:00:01:01:02:04
>> 192.168.200.1/24 2000::1/64
>> +check ovn-nbctl lsp-add internal internal-lr -- set Logical_Switch_Port
>> internal-lr \
>> +    type=router options:router-port=lr-internal
>> addresses=\"00:00:01:01:02:04\"
>> +
>> +check ovn-nbctl lsp-add internal server -- lsp-set-addresses server
>> "unknown"
>> +
>> +ovn-nbctl lsp-add public ln_port \
>> +                -- lsp-set-addresses ln_port unknown \
>> +                -- lsp-set-type ln_port localnet \
>> +                -- lsp-set-options ln_port network_name=phynet
>> +
>> +check ovn-nbctl set logical_router lr options:chassis=hv1
>> +
>> +check ovn-nbctl lb-add lb1 192.168.100.20 192.168.200.10
>> +check ovn-nbctl lb-add lb2 1000::20 2000::10
>> +check ovn-nbctl lr-lb-add lr lb1
>> +check ovn-nbctl lr-lb-add lr lb2
>> +check ovn-nbctl --wait=hv sync
>> +
>> +ADD_NAMESPACES(client)
>> +ADD_VETH(client, client, br-ext, "1000::10/64", "f0:00:00:01:02:03", \
>> +         "1000::1", "nodad", "192.168.100.10/24", "192.168.100.1")
>> +
>> +ADD_NAMESPACES(server)
>> +ADD_VETH(server, server, br-int, "2000::10/64", "f0:00:0f:01:02:03", \
>> +         "2000::1", "nodad", "192.168.200.10/24", "192.168.200.1")
>> +
>> +NETNS_DAEMONIZE([server], [nc -l -u 192.168.200.10 4242 > /dev/null],
>> [serverv4.pid])
>> +NETNS_DAEMONIZE([server], [nc -l -u 2000::10 4243 > /dev/null],
>> [serverv6.pid])
>> +
>> +NETNS_START_TCPDUMP([client], [-l -U -i client -vnne udp], [client])
>> +NETNS_START_TCPDUMP([server], [-l -U -i server -vnne udp], [server])
>> +
>> +check ovs-appctl dpctl/flush-conntrack
>> +
>> +NS_CHECK_EXEC([client], [nc -z -u 192.168.100.20 4242], [ignore],
>> [ignore], [ignore])
>> +OVS_WAIT_UNTIL([grep -q "192.168.200.10" server.tcpdump])
>> +
>> +NS_CHECK_EXEC([client], [nc -z -u 1000::20 4243])
>> +OVS_WAIT_UNTIL([grep -q "2000::10" server.tcpdump])
>> +
>> +zone_id=$(ovn-appctl -t ovn-controller ct-zone-list | grep lr_dnat | cut
>> -d ' ' -f2)
>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep -c "zone=$zone_id"],
>> [0], [2
>> +])
>> +
>> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>> +
>> +as ovn-sb
>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> +
>> +as ovn-nb
>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> +
>> +as northd
>> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
>> +
>> +as
>> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
>> +/connection dropped.*/d"])
>> +AT_CLEANUP
>> +])
>> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
>> index 5326c6e69..2ea68f212 100644
>> --- a/tests/test-ovn.c
>> +++ b/tests/test-ovn.c
>> @@ -1359,6 +1359,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx
>> OVS_UNUSED)
>>                  .group_table = &group_table,
>>                  .meter_table = &meter_table,
>>                  .collector_ids = &collector_ids,
>> +                .explicit_arp_ns_output = true,
>>
>>                  .pipeline = OVNACT_P_INGRESS,
>>                  .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
>> --
>> 2.44.0
>>
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
Thanks,
Ales
-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to