| Message ID | 20240417070907.503472-2-amusil@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v4,1/2] ci: Make sure that multinode test runs on correct branch. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Wed, Apr 17, 2024 at 9:09 AM Ales Musil <amusil@redhat.com> 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 <amusil@redhat.com> > --- > v4: Fix copy paste error in the global_handler. > v3: Rebase on top of main. > Add flag to ensure that the paused action works during upgrade. > v2: Fix the Jira link and add ack from Mark. > --- > 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 +-- > ovs | 2 +- > tests/multinode.at | 8 ++++ > tests/ovn-northd.at | 3 ++ > tests/ovn.at | 8 ++-- > tests/system-ovn.at | 95 +++++++++++++++++++++++++++++++++++++ > tests/test-ovn.c | 1 + > 16 files changed, 240 insertions(+), 63 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/ovs b/ovs > index fe55ce37a..43db93787 160000 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit fe55ce37a7b090d09dee5c01ae0797320ad678f6 > +Subproject commit 43db937876fcec57947634d6e642429673d1ec06 > 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 > > Recheck-request: github-robot-_ovn-kubernetes
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/ovs b/ovs index fe55ce37a..43db93787 160000 --- a/ovs +++ b/ovs @@ -1 +1 @@ -Subproject commit fe55ce37a7b090d09dee5c01ae0797320ad678f6 +Subproject commit 43db937876fcec57947634d6e642429673d1ec06 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,
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 <amusil@redhat.com> --- v4: Fix copy paste error in the global_handler. v3: Rebase on top of main. Add flag to ensure that the paused action works during upgrade. v2: Fix the Jira link and add ack from Mark. --- 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 +-- ovs | 2 +- tests/multinode.at | 8 ++++ tests/ovn-northd.at | 3 ++ tests/ovn.at | 8 ++-- tests/system-ovn.at | 95 +++++++++++++++++++++++++++++++++++++ tests/test-ovn.c | 1 + 16 files changed, 240 insertions(+), 63 deletions(-)