On Wed, Jan 13, 2021 at 12:58 AM Mark Michelson <[email protected]> wrote:
> On 1/12/21 4:51 AM, Numan Siddique wrote: > > > > > > On Tue, Jan 12, 2021 at 12:15 AM Mark Michelson <[email protected] > > <mailto:[email protected]>> wrote: > > > > Currently in OVN, if an SCTP packet hits a reject() action, OVN > responds > > with an ICMP packet. Instead, we should send an SCTP packet with an > > ABORT chunk. This will either end the current association or will > > prevent an association from being created, depending on which stage > of > > the SCTP state machine we currently are in. > > > > This patch adds the desired behavior for SCTP. The reject() action > will > > now send an SCTP ABORT if the incoming packet is SCTP. > > > > Signed-off-by: Mark Michelson <[email protected] > > <mailto:[email protected]>> > > > > > > Acked-by: Numan Siddique <[email protected] <mailto:[email protected]>> > > > > Please see one small nit below. I don't mind if you want to ignore it. > > > > Thanks > > Numan > > > > --- > > controller/pinctrl.c | 113 > ++++++++++++++++++++++++++++++++++++++++++ > > lib/ovn-util.h | 33 ++++++++++++ > > tests/ovn.at <http://ovn.at> | 43 ++++++++++++++++ > > utilities/ovn-trace.c | 87 ++++++++++++++++++++++++++++++++ > > 4 files changed, 276 insertions(+) > > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index 62be94018..ce11aa365 100644 > > --- a/controller/pinctrl.c > > +++ b/controller/pinctrl.c > > @@ -38,6 +38,7 @@ > > #include "openvswitch/ofp-util.h" > > #include "openvswitch/vlog.h" > > #include "lib/random.h" > > +#include "lib/crc32c.h" > > > > #include "lib/dhcp.h" > > #include "ovn-controller.h" > > @@ -1781,6 +1782,116 @@ pinctrl_handle_tcp_reset(struct rconn > > *swconn, const struct flow *ip_flow, > > dp_packet_uninit(&packet); > > } > > > > +static void dp_packet_put_sctp_abort(struct dp_packet *packet, > > + bool reflect_tag) > > +{ > > + struct sctp_chunk_header abort = { > > + .sctp_chunk_type = SCTP_CHUNK_TYPE_ABORT, > > + .sctp_chunk_flags = reflect_tag ? SCTP_ABORT_CHUNK_FLAG_T : > 0, > > + .sctp_chunk_len = htons(SCTP_CHUNK_HEADER_LEN), > > + }; > > + > > + dp_packet_put(packet, &abort, sizeof abort); > > +} > > + > > +static void > > +pinctrl_handle_sctp_abort(struct rconn *swconn, const struct flow > > *ip_flow, > > + struct dp_packet *pkt_in, > > + const struct match *md, struct ofpbuf > > *userdata, > > + bool loopback) > > +{ > > + if (ip_flow->nw_proto != IPPROTO_SCTP) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > 5); > > + VLOG_WARN_RL(&rl, "SCTP_ABORT action on non-SCTP packet"); > > + return; > > + } > > + > > + struct sctp_header *sh_in = dp_packet_l4(pkt_in); > > + if (!sh_in) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > 5); > > + VLOG_WARN_RL(&rl, "SCTP_ABORT action on malformed SCTP > > packet"); > > + return; > > + } > > + > > + const struct sctp_chunk_header *sh_in_chunk = > > + dp_packet_get_sctp_payload(pkt_in); > > + if (!sh_in_chunk) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > 5); > > + VLOG_WARN_RL(&rl, "SCTP_ABORT action on SCTP packet with no > > chunks"); > > + return; > > + } > > + > > + if (sh_in_chunk->sctp_chunk_type == SCTP_CHUNK_TYPE_ABORT) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > 5); > > + VLOG_WARN_RL(&rl, "sctp_abort action on incoming SCTP > ABORT."); > > + return; > > + } > > + > > + const struct sctp_init_chunk *sh_in_init = NULL; > > > > You can declare the above variable 'sh_in_init' inside the if since it > > is not used > > outside of this 'if'. > > Actually I can't. I use it lower in the function to help determine what > tag to use in the SCTP ABORT. I've marked it down below > Oops. Thanks for pointing out. I missed it. Thanks Numan > > > > > + if (sh_in_chunk->sctp_chunk_type == SCTP_CHUNK_TYPE_INIT) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > 5); > > + sh_in_init = dp_packet_at(pkt_in, pkt_in->l4_ofs + > > + SCTP_HEADER_LEN + > > + SCTP_CHUNK_HEADER_LEN, > > + SCTP_INIT_CHUNK_LEN); > > + if (!sh_in_init) { > > + VLOG_WARN_RL(&rl, "Incomplete SCTP INIT chunk. Ignoring > > packet."); > > + return; > > + } > > + } > > + > > + uint64_t packet_stub[128 / 8]; > > + struct dp_packet packet; > > + > > + dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub); > > + > > + struct eth_addr eth_src = loopback ? ip_flow->dl_dst : > > ip_flow->dl_src; > > + struct eth_addr eth_dst = loopback ? ip_flow->dl_src : > > ip_flow->dl_dst; > > + > > + if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) { > > + const struct in6_addr *ip6_src = > > + loopback ? &ip_flow->ipv6_dst : &ip_flow->ipv6_src; > > + const struct in6_addr *ip6_dst = > > + loopback ? &ip_flow->ipv6_src : &ip_flow->ipv6_dst; > > + pinctrl_compose_ipv6(&packet, eth_src, eth_dst, > > + (struct in6_addr *) ip6_src, > > + (struct in6_addr *) ip6_dst, > > + IPPROTO_SCTP, 63, SCTP_HEADER_LEN + > > + > SCTP_CHUNK_HEADER_LEN); > > + } else { > > + ovs_be32 nw_src = loopback ? ip_flow->nw_dst : > ip_flow->nw_src; > > + ovs_be32 nw_dst = loopback ? ip_flow->nw_src : > ip_flow->nw_dst; > > + pinctrl_compose_ipv4(&packet, eth_src, eth_dst, nw_src, > nw_dst, > > + IPPROTO_SCTP, 63, SCTP_HEADER_LEN + > > + > SCTP_CHUNK_HEADER_LEN); > > + } > > + > > + struct sctp_header *sh = dp_packet_put_zeros(&packet, sizeof > *sh); > > + dp_packet_set_l4(&packet, sh); > > + sh->sctp_dst = ip_flow->tp_src; > > + sh->sctp_src = ip_flow->tp_dst; > > + put_16aligned_be32(&sh->sctp_csum, 0); > > + > > + bool tag_reflected; > > + if (get_16aligned_be32(&sh_in->sctp_vtag) == 0 && sh_in_init) { > > Right here ^^ > If the SCTP message we receive has a 0 tag and contains an INIT chunk, > then we need to use the INIT chunk's initiate_tag in our ABORT. We use > the presence of sh_in_init to determine if the incoming SCTP message > contains an INIT chunk. > > > + /* See RFC 4960 Section 8.4, item 3. */ > > + put_16aligned_be32(&sh->sctp_vtag, > sh_in_init->initiate_tag); > > + tag_reflected = false; > > + } else { > > + /* See RFC 4960 Section 8.4, item 8. */ > > + sh->sctp_vtag = sh_in->sctp_vtag; > > + tag_reflected = true; > > + } > > + > > + dp_packet_put_sctp_abort(&packet, tag_reflected); > > + > > + put_16aligned_be32(&sh->sctp_csum, crc32c((void *) sh, > > + > > dp_packet_l4_size(&packet))); > > + > > + set_actions_and_enqueue_msg(swconn, &packet, md, userdata); > > + dp_packet_uninit(&packet); > > +} > > + > > static void > > pinctrl_handle_reject(struct rconn *swconn, const struct flow > > *ip_flow, > > struct dp_packet *pkt_in, > > @@ -1788,6 +1899,8 @@ pinctrl_handle_reject(struct rconn *swconn, > > const struct flow *ip_flow, > > { > > if (ip_flow->nw_proto == IPPROTO_TCP) { > > pinctrl_handle_tcp_reset(swconn, ip_flow, pkt_in, md, > > userdata, true); > > + } else if (ip_flow->nw_proto == IPPROTO_SCTP) { > > + pinctrl_handle_sctp_abort(swconn, ip_flow, pkt_in, md, > > userdata, true); > > } else { > > pinctrl_handle_icmp(swconn, ip_flow, pkt_in, md, userdata, > > true, true); > > } > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > > index 679f47a97..b604b073e 100644 > > --- a/lib/ovn-util.h > > +++ b/lib/ovn-util.h > > @@ -227,4 +227,37 @@ bool ip_address_and_port_from_lb_key(const char > > *key, char **ip_address, > > * value. */ > > char *ovn_get_internal_version(void); > > > > + > > +/* OVN Packet definitions. These may eventually find a home in OVS's > > + * packets.h file. For the time being, they live here because OVN > > uses them > > + * and OVS does not. > > + */ > > +#define SCTP_CHUNK_HEADER_LEN 4 > > +struct sctp_chunk_header { > > + uint8_t sctp_chunk_type; > > + uint8_t sctp_chunk_flags; > > + ovs_be16 sctp_chunk_len; > > +}; > > +BUILD_ASSERT_DECL(SCTP_CHUNK_HEADER_LEN == sizeof(struct > > sctp_chunk_header)); > > + > > +#define SCTP_INIT_CHUNK_LEN 16 > > +struct sctp_init_chunk { > > + ovs_be32 initiate_tag; > > + ovs_be32 a_rwnd; > > + ovs_be16 num_outbound_streams; > > + ovs_be16 num_inbound_streams; > > + ovs_be32 initial_tsn; > > +}; > > +BUILD_ASSERT_DECL(SCTP_INIT_CHUNK_LEN == sizeof(struct > > sctp_init_chunk)); > > + > > +/* These are the only SCTP chunk types that OVN cares about. > > + * There is no need to define the other chunk types until they are > > + * needed. > > + */ > > +#define SCTP_CHUNK_TYPE_INIT 1 > > +#define SCTP_CHUNK_TYPE_ABORT 6 > > + > > +/* See RFC 4960 Sections 3.3.7 and 8.5.1 for information on this > > flag. */ > > +#define SCTP_ABORT_CHUNK_FLAG_T (1 << 0) > > + > > #endif > > diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at < > http://ovn.at> > > index d50bdf9f7..c71e81822 100644 > > --- a/tests/ovn.at <http://ovn.at> > > +++ b/tests/ovn.at <http://ovn.at> > > @@ -12895,6 +12895,45 @@ test_tcp_syn_packet() { > > check as hv$hv ovs-appctl netdev-dummy/receive vif$inport > $packet > > } > > > > +# test_sctp_init_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST > > IP_CHKSUM SCTP_SPORT SCTP_DPORT SCTP_INIT_TAG SCTP_CHKSUM > > EXP_IP_CHKSUM EXP_SCTP_ABORT_CHKSUM > > +# > > +# Causes a packet to be received on INPORT of the hypervisor HV. > > The packet is an SCTP INIT chunk with > > +# ETH_SRC, ETH_DST, IPV4_SRC, IPV4_DST, IP_CHKSUM, SCTP_SPORT, > > SCTP_DPORT, and SCTP_CHKSUM as specified. > > +# The INIT "initiate_tag" will be set to SCTP_INIT_TAG. > > +# EXP_IP_CHKSUM and EXP_SCTP_CHKSUM are the ip and sctp checksums > > of the SCTP ABORT chunk generated from the ACL rule hit > > +# > > +# INPORT is an lport number, e.g. 11 for vif11. > > +# HV is a hypervisor number. > > +# ETH_SRC and ETH_DST are each 12 hex digits. > > +# IPV4_SRC and IPV4_DST are each 8 hex digits. > > +# SCTP_SPORT and SCTP_DPORT are 4 hex digits. > > +# IP_CHKSUM and EXP_IP_CHKSUM are 4 hex digits. > > +# SCTP_CHKSUM and EXP_SCTP_CHKSUM are 8 hex digits. > > +test_sctp_init_packet() { > > + local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 > > ipv4_dst=$6 ip_chksum=$7 > > + local sctp_sport=$8 sctp_dport=$9 sctp_init_tag=${10} > > sctp_chksum=${11} > > + local exp_ip_chksum=${12} exp_sctp_abort_chksum=${13} > > + > > + local ip_ttl=ff > > + local eth_hdr=${eth_dst}${eth_src}0800 > > + local > > ip_hdr=4500002500004000${ip_ttl}84${ip_chksum}${ipv4_src}${ipv4_dst} > > + local sctp_hdr=${sctp_sport}${sctp_dport}00000000${sctp_chksum} > > + local > > sctp_init=01000014${sctp_init_tag}0000000000010001${sctp_init_tag} > > + > > + local packet=${eth_hdr}${ip_hdr}${sctp_hdr}${sctp_init} > > + > > + local sctp_abort_ttl=3f > > + local reply_eth_hdr=${eth_src}${eth_dst}0800 > > + local > > > > reply_ip_hdr=4500002400004000${sctp_abort_ttl}84${exp_ip_chksum}${ipv4_dst}${ipv4_src} > > + local > > > > reply_sctp_hdr=${sctp_dport}${sctp_sport}${sctp_init_tag}${exp_sctp_abort_chksum} > > + local reply_sctp_abort=06000004 > > + > > + local > > > reply=${reply_eth_hdr}${reply_ip_hdr}${reply_sctp_hdr}${reply_sctp_abort} > > + echo $reply >> vif$inport.expected > > + > > + check as hv$hv ovs-appctl netdev-dummy/receive vif$inport > $packet > > +} > > + > > # Create hypervisors hv[123]. > > # Add vif1[123] to hv1, vif2[123] to hv2, vif3[123] to hv3. > > # Add all of the vifs to a single logical switch sw0. > > @@ -12948,6 +12987,10 @@ test_tcp_syn_packet 11 1 000000000011 > > 000000000021 $(ip_to_hex 192 168 1 11) $(i > > test_tcp_syn_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 > > 168 1 21) $(ip_to_hex 192 168 1 11) 0000 8b40 3039 0000 b85f 70e4 > > test_tcp_syn_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 > > 168 1 31) $(ip_to_hex 192 168 1 12) 0000 8b40 3039 0000 b854 70d9 > > > > +test_sctp_init_packet 11 1 000000000011 000000000021 $(ip_to_hex > > 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 8b40 3039 00000001 > > 82112601 b7e5 10fe95b6 > > +test_sctp_init_packet 21 2 000000000021 000000000011 $(ip_to_hex > > 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 8b40 3039 00000002 > > C0379D5A b7e5 39f23aaf > > +test_sctp_init_packet 31 3 000000000031 000000000012 $(ip_to_hex > > 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 8b40 3039 00000003 > > 028E263C b7da 7124045b > > + > > for i in 1 2 3; do > > OVN_CHECK_PACKETS([hv$i/vif${i}1-tx.pcap], [vif${i}1.expected]) > > done > > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > > index a81a5e936..d54ffbcd9 100644 > > --- a/utilities/ovn-trace.c > > +++ b/utilities/ovn-trace.c > > @@ -1806,6 +1806,91 @@ execute_tcp_reset(const struct ovnact_nest > *on, > > execute_tcp6_reset(on, dp, uflow, table_id, loopback, > > pipeline, super); > > } > > } > > + > > +static void > > +execute_sctp4_abort(const struct ovnact_nest *on, > > + const struct ovntrace_datapath *dp, > > + const struct flow *uflow, uint8_t table_id, > > + bool loopback, enum ovnact_pipeline pipeline, > > + struct ovs_list *super) > > +{ > > + struct flow sctp_flow = *uflow; > > + > > + /* Update fields for TCP SCTP. */ > > + if (loopback) { > > + sctp_flow.dl_dst = uflow->dl_src; > > + sctp_flow.dl_src = uflow->dl_dst; > > + sctp_flow.nw_dst = uflow->nw_src; > > + sctp_flow.nw_src = uflow->nw_dst; > > + } else { > > + sctp_flow.dl_dst = uflow->dl_dst; > > + sctp_flow.dl_src = uflow->dl_src; > > + sctp_flow.nw_dst = uflow->nw_dst; > > + sctp_flow.nw_src = uflow->nw_src; > > + } > > + sctp_flow.nw_proto = IPPROTO_SCTP; > > + sctp_flow.nw_ttl = 255; > > + sctp_flow.tp_src = uflow->tp_src; > > + sctp_flow.tp_dst = uflow->tp_dst; > > + > > + struct ovntrace_node *node = ovntrace_node_append( > > + super, OVNTRACE_NODE_TRANSFORMATION, "sctp_abort"); > > + > > + trace_actions(on->nested, on->nested_len, dp, &sctp_flow, > > + table_id, pipeline, &node->subs); > > +} > > + > > +static void > > +execute_sctp6_abort(const struct ovnact_nest *on, > > + const struct ovntrace_datapath *dp, > > + const struct flow *uflow, uint8_t table_id, > > + bool loopback, enum ovnact_pipeline pipeline, > > + struct ovs_list *super) > > +{ > > + struct flow sctp_flow = *uflow; > > + > > + /* Update fields for SCTP. */ > > + if (loopback) { > > + sctp_flow.dl_dst = uflow->dl_src; > > + sctp_flow.dl_src = uflow->dl_dst; > > + sctp_flow.ipv6_dst = uflow->ipv6_src; > > + sctp_flow.ipv6_src = uflow->ipv6_dst; > > + } else { > > + sctp_flow.dl_dst = uflow->dl_dst; > > + sctp_flow.dl_src = uflow->dl_src; > > + sctp_flow.ipv6_dst = uflow->ipv6_dst; > > + sctp_flow.ipv6_src = uflow->ipv6_src; > > + } > > + sctp_flow.nw_proto = IPPROTO_TCP; > > + sctp_flow.nw_ttl = 255; > > + sctp_flow.tp_src = uflow->tp_src; > > + sctp_flow.tp_dst = uflow->tp_dst; > > + sctp_flow.tcp_flags = htons(TCP_RST); > > + > > + struct ovntrace_node *node = ovntrace_node_append( > > + super, OVNTRACE_NODE_TRANSFORMATION, "sctp_abort"); > > + > > + trace_actions(on->nested, on->nested_len, dp, &sctp_flow, > > + table_id, pipeline, &node->subs); > > +} > > + > > +static void > > +execute_sctp_abort(const struct ovnact_nest *on, > > + const struct ovntrace_datapath *dp, > > + const struct flow *uflow, uint8_t table_id, > > + bool loopback, enum ovnact_pipeline pipeline, > > + struct ovs_list *super) > > +{ > > + if (get_dl_type(uflow) == htons(ETH_TYPE_IP)) { > > + execute_sctp4_abort(on, dp, uflow, table_id, loopback, > > + pipeline, super); > > + } else { > > + execute_sctp6_abort(on, dp, uflow, table_id, loopback, > > + pipeline, super); > > + } > > +} > > + > > + > > static void > > execute_reject(const struct ovnact_nest *on, > > const struct ovntrace_datapath *dp, > > @@ -1814,6 +1899,8 @@ execute_reject(const struct ovnact_nest *on, > > { > > if (uflow->nw_proto == IPPROTO_TCP) { > > execute_tcp_reset(on, dp, uflow, table_id, true, pipeline, > > super); > > + } else if (uflow->nw_proto == IPPROTO_SCTP) { > > + execute_sctp_abort(on, dp, uflow, table_id, true, pipeline, > > super); > > } else { > > if (get_dl_type(uflow) == htons(ETH_TYPE_IP)) { > > execute_icmp4(on, dp, uflow, table_id, true, pipeline, > > super); > > -- > > 2.29.2 > > > > _______________________________________________ > > dev mailing list > > [email protected] <mailto:[email protected]> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
