| Message ID | 20200428110609.3680194-1-numans@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,ovn] Fix ACL reject action for UDP packets. | expand |
> > From: Numan Siddique <numans@ovn.org> > > The icmp packet generated by ovn-controller for reject ACL action > for non TCP packets is not getting delivered to the sender of > the original packet. This is because the icmp packets are skipped > from out_pre_lb/out_pre_acl logical switch egress pipeline and this > results in these icmp packets getting dropped in the ACL stage because > of invalid ct flags. This patch fixes this issue by removing those logical > flows. The IP checksum generated by ovn-controller is invalid. This patch > fixes this issue as well. > > Signed-off-by: Numan Siddique <numans@ovn.org> Tested-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/pinctrl.c | 102 ++++++++++++++++++++++++++++--------------- > northd/ovn-northd.c | 22 +++++----- > tests/ovn.at | 46 +++++++++---------- > tests/system-ovn.at | 95 ++++++++++++++++++++++++++++++++-------- > 4 files changed, 177 insertions(+), 88 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 3230bb386..63796d88c 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -1453,7 +1453,7 @@ static void > pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, > struct dp_packet *pkt_in, > const struct match *md, struct ofpbuf *userdata, > - bool include_orig_ip_datagram) > + bool set_icmp_code) > { > /* 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. */ > @@ -1500,46 +1500,51 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, > packet_set_ipv4(&packet, ip_flow->nw_src, ip_flow->nw_dst, > ip_flow->nw_tos, 255); > > + uint8_t icmp_code = 1; > + if (set_icmp_code && in_ip->ip_proto == IPPROTO_UDP) { > + icmp_code = 3; > + } > + > struct icmp_header *ih = dp_packet_put_zeros(&packet, sizeof *ih); > dp_packet_set_l4(&packet, ih); > - packet_set_icmp(&packet, ICMP4_DST_UNREACH, 1); > - > - if (include_orig_ip_datagram) { > - /* RFC 1122: 3.2.2 MUST send at least the IP header and 8 bytes > - * of header. MAY send more. > - * RFC says return as much as we can without exceeding 576 > - * bytes. > - * So, lets return as much as we can. */ > - > - /* Calculate available room to include the original IP + data. */ > - nh = dp_packet_l3(&packet); > - uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len)); > - if (in_ip_len > room) { > - in_ip_len = room; > - } > - dp_packet_put(&packet, in_ip, in_ip_len); > - > - /* dp_packet_put may reallocate the buffer. Get the l3 and l4 > - * header pointers again. */ > - nh = dp_packet_l3(&packet); > - ih = dp_packet_l4(&packet); > - uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len; > - nh->ip_tot_len = htons(ip_total_len); > - ih->icmp_csum = 0; > - ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len); > - nh->ip_csum = 0; > - nh->ip_csum = csum(nh, sizeof *nh); > - } > + packet_set_icmp(&packet, ICMP4_DST_UNREACH, icmp_code); > + > + /* RFC 1122: 3.2.2 MUST send at least the IP header and 8 bytes > + * of header. MAY send more. > + * RFC says return as much as we can without exceeding 576 > + * bytes. > + * So, lets return as much as we can. */ > + > + /* Calculate available room to include the original IP + data. */ > + nh = dp_packet_l3(&packet); > + uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len)); > + if (in_ip_len > room) { > + in_ip_len = room; > + } > + dp_packet_put(&packet, in_ip, in_ip_len); > + > + /* dp_packet_put may reallocate the buffer. Get the l3 and l4 > + * header pointers again. */ > + nh = dp_packet_l3(&packet); > + ih = dp_packet_l4(&packet); > + uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len; > + nh->ip_tot_len = htons(ip_total_len); > + ih->icmp_csum = 0; > + ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len); > + nh->ip_csum = 0; > + nh->ip_csum = csum(nh, sizeof *nh); > + > } else { > struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh); > struct icmp6_data_header *ih; > uint32_t icmpv6_csum; > + struct ip6_hdr *in_ip = dp_packet_l3(pkt_in); > > eh->eth_type = htons(ETH_TYPE_IPV6); > dp_packet_set_l3(&packet, nh); > nh->ip6_vfc = 0x60; > nh->ip6_nxt = IPPROTO_ICMPV6; > - nh->ip6_plen = htons(sizeof(*nh) + ICMP6_DATA_HEADER_LEN); > + nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN); > packet_set_ipv6(&packet, &ip_flow->ipv6_src, &ip_flow->ipv6_dst, > ip_flow->nw_tos, ip_flow->ipv6_label, 255); > > @@ -1547,15 +1552,42 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, > dp_packet_set_l4(&packet, ih); > ih->icmp6_base.icmp6_type = ICMP6_DST_UNREACH; > ih->icmp6_base.icmp6_code = 1; > + > + if (set_icmp_code && in_ip->ip6_nxt == IPPROTO_UDP) { > + ih->icmp6_base.icmp6_code = ICMP6_DST_UNREACH_NOPORT; > + } > ih->icmp6_base.icmp6_cksum = 0; > > - uint8_t *data = dp_packet_put_zeros(&packet, sizeof *nh); > - memcpy(data, dp_packet_l3(pkt_in), sizeof(*nh)); > + nh = dp_packet_l3(&packet); > + > + /* RFC 4443: 3.1. > + * > + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > + * | Type | Code | Checksum | > + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > + * | Unused | > + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > + * | As much of invoking packet | > + * + as possible without the ICMPv6 packet + > + * | exceeding the minimum IPv6 MTU [IPv6] | > + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > + */ > + > + uint16_t room = 1280 - (sizeof *eh + sizeof *nh + > + ICMP6_DATA_HEADER_LEN); > + uint16_t in_ip_len = (uint16_t) sizeof *in_ip + ntohs(in_ip->ip6_plen); > + if (in_ip_len > room) { > + in_ip_len = room; > + } > + > + dp_packet_put(&packet, in_ip, in_ip_len); > + nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN + in_ip_len); > > icmpv6_csum = packet_csum_pseudoheader6(dp_packet_l3(&packet)); > ih->icmp6_base.icmp6_cksum = csum_finish( > csum_continue(icmpv6_csum, ih, > - sizeof(*nh) + ICMP6_DATA_HEADER_LEN)); > + in_ip_len + ICMP6_DATA_HEADER_LEN)); > } > > if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) { > @@ -2646,12 +2678,12 @@ process_packet_in(struct rconn *swconn, const struct ofp_header *msg) > > case ACTION_OPCODE_ICMP: > pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, > - &userdata, false); > + &userdata, true); > break; > > case ACTION_OPCODE_ICMP4_ERROR: > pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, > - &userdata, true); > + &userdata, false); > break; > > case ACTION_OPCODE_TCP_RESET: > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 63753ac61..eb459c8c4 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -4724,12 +4724,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) > * Not to do conntrack on ND and ICMP destination > * unreachable packets. */ > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, > - "nd || nd_rs || nd_ra || icmp4.type == 3 || " > - "icmp6.type == 1 || " > + "nd || nd_rs || nd_ra || " > "(udp && udp.src == 546 && udp.dst == 547)", "next;"); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, > - "nd || nd_rs || nd_ra || icmp4.type == 3 || " > - "icmp6.type == 1 || " > + "nd || nd_rs || nd_ra || " > "(udp && udp.src == 546 && udp.dst == 547)", "next;"); > > /* Ingress and Egress Pre-ACL Table (Priority 100). > @@ -4841,12 +4839,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, > { > /* Do not send ND packets to conntrack */ > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, > - "nd || nd_rs || nd_ra || icmp4.type == 3 ||" > - "icmp6.type == 1", > + "nd || nd_rs || nd_ra", > "next;"); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, > - "nd || nd_rs || nd_ra || icmp4.type == 3 ||" > - "icmp6.type == 1", > + "nd || nd_rs || nd_ra", > "next;"); > > /* Do not send service monitor packets to conntrack. */ > @@ -5025,9 +5021,10 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, > ds_put_format(&actions, "%s ", extra_actions->string); > } > ds_put_format(&actions, "reg0 = 0; " > - "eth.dst <-> eth.src; ip4.dst <-> ip4.src; " > - "icmp4 { outport <-> inport; %s };", > - ingress ? "output;" : "next(pipeline=ingress,table=0);"); > + "icmp4 { eth.dst <-> eth.src; ip4.dst <-> ip4.src; " > + "outport <-> inport; %s };", > + ingress ? "next(pipeline=egress,table=5);" > + : "next(pipeline=ingress,table=19);"); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > ds_cstr(&match), ds_cstr(&actions), stage_hint); > @@ -5044,7 +5041,8 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, > ds_put_format(&actions, "reg0 = 0; icmp6 { " > "eth.dst <-> eth.src; ip6.dst <-> ip6.src; " > "outport <-> inport; %s };", > - ingress ? "output;" : "next(pipeline=ingress,table=0);"); > + ingress ? "next(pipeline=egress,table=5);" > + : "next(pipeline=ingress,table=19);"); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > ds_cstr(&match), ds_cstr(&actions), stage_hint); > diff --git a/tests/ovn.at b/tests/ovn.at > index e6febd4c2..6467bdc42 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -11813,13 +11813,13 @@ test_ip_packet() { > > local ip_ttl=ff > local packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > - > + local orig_pkt_in_reply=4500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > local reply_icmp_ttl=ff > local icmp_type_code_response=0301 > local icmp_data=00000000 > local reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} > - local reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload} > - echo $reply >> vif$inport.expected > + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload} > + echo $reply$orig_pkt_in_reply >> vif$inport.expected > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > } > @@ -11836,7 +11836,7 @@ test_ipv6_packet() { > local ip6_hdr=6000000000083aff${ipv6_src}${ipv6_dst} > local packet=${eth_dst}${eth_src}86dd${ip6_hdr}0000000000000000 > > - local reply=${eth_src}${eth_dst}86dd6000000000303aff${ipv6_dst}${ipv6_src}0101${exp_icmp_chksum}00000000${ip6_hdr} > + local reply=${eth_src}${eth_dst}86dd6000000000383aff${ipv6_dst}${ipv6_src}0101${exp_icmp_chksum}00000000${ip6_hdr}0000000000000000 > echo $reply >> vif$inport.expected > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > @@ -11914,11 +11914,11 @@ ovn-nbctl --log acl-add sw0 from-lport 1000 "inport == \"sw0-p21\"" reject > # Allow some time for ovn-northd and ovn-controller to catch up. > ovn-nbctl --timeout=3 --wait=hv sync > > -test_ip_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 7d8d fcfe > -test_ip_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 7d8d fcfe > -test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 7d82 fcfe > +test_ip_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 f85b f576 > +test_ip_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 f85b f576 > +test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 f850 f56b > > -test_ipv6_packet 11 1 000000000011 000000000021 fe80000000000000020001fffe000001 fe80000000000000020001fffe000002 6183 > +test_ipv6_packet 11 1 000000000011 000000000021 fe80000000000000020001fffe000001 fe80000000000000020001fffe000002 617b > > test_tcp_syn_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 8b40 3039 0000 b85f 70e4 > 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 > @@ -12871,13 +12871,13 @@ test_ip_packet() { > > local ip_ttl=01 > local packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > - > + local orig_pkt_in_reply=4500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > local reply_icmp_ttl=fe > local icmp_type_code_response=0b00 > local icmp_data=00000000 > local reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} > - local reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > - echo $reply >> vif$inport.expected > + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > + echo $reply$orig_pkt_in_reply >> vif$inport.expected > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > } > @@ -12895,7 +12895,7 @@ test_ip6_packet() { > local ip6_hdr=6000000000151101${ipv6_src}${ipv6_dst} > local packet=${eth_dst}${eth_src}86dd${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a > > - local reply=${eth_src}${eth_dst}86dd6000000000303afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr} > + local reply=${eth_src}${eth_dst}86dd6000000000453afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a > echo $reply >> vif$inport.expected > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > @@ -12937,8 +12937,8 @@ OVN_POPULATE_ARP > # allow some time for ovn-northd and ovn-controller to catch up. > ovn-nbctl --wait=hv sync > > -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 7dae f4ff > -test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 d461 > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 f87c ea96 > +test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 1c22 > OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) > > OVN_CLEANUP([hv1], [hv2]) > @@ -12967,12 +12967,12 @@ test_ip_packet() { > > local ip_ttl=ff > local packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}${l4_proto}${ip_chksum}${ipv4_src}${ip_router} > - > + local orig_pkt_in_reply=4500001400004000${ip_ttl}${l4_proto}${ip_chksum}${ipv4_src}${ip_router} > local reply_icmp_ttl=fe > local icmp_data=00000000 > local reply_icmp_payload=${exp_icmp_code}${exp_icmp_chksum}${icmp_data} > - local reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > - echo $reply >> vif$inport.expected > + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > + echo $reply$orig_pkt_in_reply >> vif$inport.expected > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > } > @@ -13038,7 +13038,9 @@ test_ip6_packet() { > local ip6_hdr=60000000${ipv6_len}${ipv6_proto}ff${ipv6_src}${ipv6_dst} > local packet=${eth_dst}${eth_src}86dd${ip6_hdr}${data} > > - local reply=${eth_src}${eth_dst}86dd6000000000303afe${ipv6_dst}${ipv6_src}${exp_icmp_code}${exp_icmp_chksum}00000000${ip6_hdr} > + local reply_ip_len=`expr 48 + ${#data} / 2` > + reply_ip_len=$(printf "%x" $reply_ip_len) > + local reply=${eth_src}${eth_dst}86dd6000000000${reply_ip_len}3afe${ipv6_dst}${ipv6_src}${exp_icmp_code}${exp_icmp_chksum}00000000${ip6_hdr}${data} > echo $reply >> vif$inport.expected > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > @@ -13080,13 +13082,13 @@ OVN_POPULATE_ARP > # allow some time for ovn-northd and ovn-controller to catch up. > ovn-nbctl --wait=hv sync > > -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 11 0000 7dae fcfc 0303 > -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 84 0000 7dae fcfd 0302 > -test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015 dbb8303900155bac6b646f65206676676e6d66720a 0104 d570 > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 11 0000 f87c f485 0303 > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 84 0000 f87c f413 0302 > +test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015 dbb8303900155bac6b646f65206676676e6d66720a 0104 1d31 > OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) > > test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 b680 6e05 > -test_ip6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004 01020304 0103 627e > +test_ip6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004 01020304 0103 5e74 > test_tcp6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 8b40 3039 0000 98cd > OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [vif2.expected]) > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index fa3b83cb1..117f1e835 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -3697,7 +3697,7 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > AT_CLEANUP > > > -AT_SETUP([ovn -- ACL reject - TCP reset]) > +AT_SETUP([ovn -- ACL reject]) > AT_SKIP_IF([test $HAVE_NC = no]) > AT_KEYWORDS([lb]) > > @@ -3736,13 +3736,14 @@ ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" drop > ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" drop > > ovn-nbctl pg-add pg0 sw0-p1-rej sw0-p2-rej > -ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" allow-related > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip" allow-related > ovn-nbctl --log acl-add pg0 from-lport 1004 "inport == @pg0 && ip && tcp && tcp.dst == 80" reject > +ovn-nbctl --log acl-add pg0 from-lport 1004 "inport == @pg0 && ip && udp && udp.dst == 90" reject > > -ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related > ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 82" allow-related > ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 82" allow-related > ovn-nbctl --log acl-add pg0 to-lport 1004 "inport == @pg0 && ip && tcp && tcp.dst == 84" reject > +ovn-nbctl --log acl-add pg0 to-lport 1004 "inport == @pg0 && ip && udp && udp.dst == 94" reject > > OVN_POPULATE_ARP > ovn-nbctl --wait=hv sync > @@ -3758,33 +3759,38 @@ ADD_VETH(sw0-p2-rej, sw0-p2-rej, br-int, "10.0.0.4/24", "50:54:00:00:00:04", \ > NS_CHECK_EXEC([sw0-p1-rej], [ip a a aef0::3/64 dev sw0-p1-rej], [0]) > NS_CHECK_EXEC([sw0-p2-rej], [ip a a aef0::4/64 dev sw0-p2-rej], [0]) > > -# Capture packets in sw0-p1-rej. > -NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 2 -i sw0-p1-rej tcp port 80 > sw0-p1-rej-ip4.pcap &], [0]) > sleep 1 > > -NS_CHECK_EXEC([sw0-p1-rej], [nc 10.0.0.4 80], [1], [], > -[dnl > -Ncat: Connection refused. > -]) > +# Capture packets in sw0-p1-rej. > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 4 -i sw0-p1-rej tcp > sw0-p1-rej-ip4.pcap &], [0]) > + > +sleep 1 > > OVS_WAIT_UNTIL([ > - total=`cat sw0-p1-rej-ip4.pcap | wc -l` > - echo "total = $total" > - test "${total}" = "2" > + ip netns exec sw0-p1-rej nc 10.0.0.4 80 2> r > + res=$(cat r) > + test "$res" = "Ncat: Connection refused." > ]) > > # Now send traffic to port 84 > -NS_CHECK_EXEC([sw0-p1-rej], [nc 10.0.0.4 84], [1], [], > -[dnl > -Ncat: Connection refused. > +OVS_WAIT_UNTIL([ > + ip netns exec sw0-p1-rej nc 10.0.0.4 84 2> r > + res=$(cat r) > + test "$res" = "Ncat: Connection refused." > ]) > > -AT_CHECK([ > +OVS_WAIT_UNTIL([ > n_pkt=$(ovs-ofctl dump-flows br-int table=44 | grep -v n_packets=0 | \ > grep controller | grep tp_dst=84 -c) > test $n_pkt -eq 1 > ]) > > +OVS_WAIT_UNTIL([ > + total=`cat sw0-p1-rej-ip4.pcap | wc -l` > + echo "total = $total" > + test "${total}" = "4" > +]) > + > # Without this sleep, test case fails intermittently. > sleep 3 > > @@ -3792,17 +3798,68 @@ NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 2 -i sw0-p2-rej tcp port 80 > sw0-p2- > > sleep 1 > > -NS_CHECK_EXEC([sw0-p2-rej], [nc -6 aef0::3 80], [1], [], > -[dnl > -Ncat: Connection refused. > +OVS_WAIT_UNTIL([ > + ip netns exec sw0-p2-rej nc -6 aef0::3 80 2> r > + res=$(cat r) > + test "$res" = "Ncat: Connection refused." > ]) > > + > OVS_WAIT_UNTIL([ > total=`cat sw0-p2-rej-ip6.pcap | wc -l` > echo "total = $total" > test "${total}" = "2" > ]) > > +# Now test for IPv4 UDP. > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 90 > sw0-p1-rej-udp.pcap &], [0]) > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp > sw0-p1-rej-icmp.pcap &], [0]) > + > +echo "foo" > foo > +OVS_WAIT_UNTIL([ > + ip netns exec sw0-p1-rej nc -u 10.0.0.4 90 < foo > + c=$(cat sw0-p1-rej-icmp.pcap | grep \ > +"10.0.0.4 > 10.0.0.3: ICMP 10.0.0.4 udp port dnsix unreachable" | uniq | wc -l) > + test $c -eq 1 > +]) > + > +rm -f *.pcap > + > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 94 > sw0-p1-rej-udp.pcap &], [0]) > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp > sw0-p1-rej-icmp.pcap &], [0]) > + > +OVS_WAIT_UNTIL([ > + ip netns exec sw0-p1-rej nc -u 10.0.0.4 94 < foo > + c=$(cat sw0-p1-rej-icmp.pcap | grep \ > +"10.0.0.4 > 10.0.0.3: ICMP 10.0.0.4 udp port objcall unreachable" | uniq | wc -l) > + test $c -eq 1 > +]) > + > +# Now test for IPv6 UDP. > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej udp port 90 > sw0-p2-rej-ip6-udp.pcap &], [0]) > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej icmp6 > sw0-p2-rej-icmp6.pcap &], [0]) > + > +OVS_WAIT_UNTIL([ > + ip netns exec sw0-p2-rej nc -u -6 aef0::3 90 < foo > + c=$(cat sw0-p2-rej-icmp6.pcap | grep \ > +"IP6 aef0::3 > aef0::4: ICMP6, destination unreachable, unreachable port, \ > +aef0::3 udp port dnsix" | uniq | wc -l) > + test $c -eq 1 > +]) > + > +rm -f *.pcap > + > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej udp port 94 > sw0-p2-rej-ip6-udp.pcap &], [0]) > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej icmp6 > sw0-p2-rej-icmp6.pcap &], [0]) > + > +OVS_WAIT_UNTIL([ > + ip netns exec sw0-p2-rej nc -u -6 aef0::3 94 < foo > + c=$(cat sw0-p2-rej-icmp6.pcap | grep \ > +"IP6 aef0::3 > aef0::4: ICMP6, destination unreachable, unreachable port, \ > +aef0::3 udp port objcall" | uniq | wc -l) > + test $c -eq 1 > +]) > + > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > as ovn-sb > -- > 2.25.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, May 5, 2020 at 6:34 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > > > From: Numan Siddique <numans@ovn.org> > > > > The icmp packet generated by ovn-controller for reject ACL action > > for non TCP packets is not getting delivered to the sender of > > the original packet. This is because the icmp packets are skipped > > from out_pre_lb/out_pre_acl logical switch egress pipeline and this > > results in these icmp packets getting dropped in the ACL stage because > > of invalid ct flags. This patch fixes this issue by removing those > logical > > flows. The IP checksum generated by ovn-controller is invalid. This patch > > fixes this issue as well. > > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > Tested-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > Thanks Lorenzo. I applied this patch to master and branch-20.03. Numan > > > --- > > controller/pinctrl.c | 102 ++++++++++++++++++++++++++++--------------- > > northd/ovn-northd.c | 22 +++++----- > > tests/ovn.at | 46 +++++++++---------- > > tests/system-ovn.at | 95 ++++++++++++++++++++++++++++++++-------- > > 4 files changed, 177 insertions(+), 88 deletions(-) > > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index 3230bb386..63796d88c 100644 > > --- a/controller/pinctrl.c > > +++ b/controller/pinctrl.c > > @@ -1453,7 +1453,7 @@ static void > > pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, > > struct dp_packet *pkt_in, > > const struct match *md, struct ofpbuf *userdata, > > - bool include_orig_ip_datagram) > > + bool set_icmp_code) > > { > > /* 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. */ > > @@ -1500,46 +1500,51 @@ pinctrl_handle_icmp(struct rconn *swconn, const > struct flow *ip_flow, > > packet_set_ipv4(&packet, ip_flow->nw_src, ip_flow->nw_dst, > > ip_flow->nw_tos, 255); > > > > + uint8_t icmp_code = 1; > > + if (set_icmp_code && in_ip->ip_proto == IPPROTO_UDP) { > > + icmp_code = 3; > > + } > > + > > struct icmp_header *ih = dp_packet_put_zeros(&packet, sizeof > *ih); > > dp_packet_set_l4(&packet, ih); > > - packet_set_icmp(&packet, ICMP4_DST_UNREACH, 1); > > - > > - if (include_orig_ip_datagram) { > > - /* RFC 1122: 3.2.2 MUST send at least the IP header and 8 > bytes > > - * of header. MAY send more. > > - * RFC says return as much as we can without exceeding 576 > > - * bytes. > > - * So, lets return as much as we can. */ > > - > > - /* Calculate available room to include the original IP + > data. */ > > - nh = dp_packet_l3(&packet); > > - uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len)); > > - if (in_ip_len > room) { > > - in_ip_len = room; > > - } > > - dp_packet_put(&packet, in_ip, in_ip_len); > > - > > - /* dp_packet_put may reallocate the buffer. Get the l3 and > l4 > > - * header pointers again. */ > > - nh = dp_packet_l3(&packet); > > - ih = dp_packet_l4(&packet); > > - uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len; > > - nh->ip_tot_len = htons(ip_total_len); > > - ih->icmp_csum = 0; > > - ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len); > > - nh->ip_csum = 0; > > - nh->ip_csum = csum(nh, sizeof *nh); > > - } > > + packet_set_icmp(&packet, ICMP4_DST_UNREACH, icmp_code); > > + > > + /* RFC 1122: 3.2.2 MUST send at least the IP header and 8 > bytes > > + * of header. MAY send more. > > + * RFC says return as much as we can without exceeding 576 > > + * bytes. > > + * So, lets return as much as we can. */ > > + > > + /* Calculate available room to include the original IP + data. > */ > > + nh = dp_packet_l3(&packet); > > + uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len)); > > + if (in_ip_len > room) { > > + in_ip_len = room; > > + } > > + dp_packet_put(&packet, in_ip, in_ip_len); > > + > > + /* dp_packet_put may reallocate the buffer. Get the l3 and l4 > > + * header pointers again. */ > > + nh = dp_packet_l3(&packet); > > + ih = dp_packet_l4(&packet); > > + uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len; > > + nh->ip_tot_len = htons(ip_total_len); > > + ih->icmp_csum = 0; > > + ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len); > > + nh->ip_csum = 0; > > + nh->ip_csum = csum(nh, sizeof *nh); > > + > > } else { > > struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh); > > struct icmp6_data_header *ih; > > uint32_t icmpv6_csum; > > + struct ip6_hdr *in_ip = dp_packet_l3(pkt_in); > > > > eh->eth_type = htons(ETH_TYPE_IPV6); > > dp_packet_set_l3(&packet, nh); > > nh->ip6_vfc = 0x60; > > nh->ip6_nxt = IPPROTO_ICMPV6; > > - nh->ip6_plen = htons(sizeof(*nh) + ICMP6_DATA_HEADER_LEN); > > + nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN); > > packet_set_ipv6(&packet, &ip_flow->ipv6_src, &ip_flow->ipv6_dst, > > ip_flow->nw_tos, ip_flow->ipv6_label, 255); > > > > @@ -1547,15 +1552,42 @@ pinctrl_handle_icmp(struct rconn *swconn, const > struct flow *ip_flow, > > dp_packet_set_l4(&packet, ih); > > ih->icmp6_base.icmp6_type = ICMP6_DST_UNREACH; > > ih->icmp6_base.icmp6_code = 1; > > + > > + if (set_icmp_code && in_ip->ip6_nxt == IPPROTO_UDP) { > > + ih->icmp6_base.icmp6_code = ICMP6_DST_UNREACH_NOPORT; > > + } > > ih->icmp6_base.icmp6_cksum = 0; > > > > - uint8_t *data = dp_packet_put_zeros(&packet, sizeof *nh); > > - memcpy(data, dp_packet_l3(pkt_in), sizeof(*nh)); > > + nh = dp_packet_l3(&packet); > > + > > + /* RFC 4443: 3.1. > > + * > > + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 > 0 1 > > + * > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + * | Type | Code | Checksum > | > > + * > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + * | Unused > | > > + * > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + * | As much of invoking packet > | > > + * + as possible without the ICMPv6 packet > + > > + * | exceeding the minimum IPv6 MTU [IPv6] > | > > + * > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + */ > > + > > + uint16_t room = 1280 - (sizeof *eh + sizeof *nh + > > + ICMP6_DATA_HEADER_LEN); > > + uint16_t in_ip_len = (uint16_t) sizeof *in_ip + > ntohs(in_ip->ip6_plen); > > + if (in_ip_len > room) { > > + in_ip_len = room; > > + } > > + > > + dp_packet_put(&packet, in_ip, in_ip_len); > > + nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN + in_ip_len); > > > > icmpv6_csum = packet_csum_pseudoheader6(dp_packet_l3(&packet)); > > ih->icmp6_base.icmp6_cksum = csum_finish( > > csum_continue(icmpv6_csum, ih, > > - sizeof(*nh) + ICMP6_DATA_HEADER_LEN)); > > + in_ip_len + ICMP6_DATA_HEADER_LEN)); > > } > > > > if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) { > > @@ -2646,12 +2678,12 @@ process_packet_in(struct rconn *swconn, const > struct ofp_header *msg) > > > > case ACTION_OPCODE_ICMP: > > pinctrl_handle_icmp(swconn, &headers, &packet, > &pin.flow_metadata, > > - &userdata, false); > > + &userdata, true); > > break; > > > > case ACTION_OPCODE_ICMP4_ERROR: > > pinctrl_handle_icmp(swconn, &headers, &packet, > &pin.flow_metadata, > > - &userdata, true); > > + &userdata, false); > > break; > > > > case ACTION_OPCODE_TCP_RESET: > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 63753ac61..eb459c8c4 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -4724,12 +4724,10 @@ build_pre_acls(struct ovn_datapath *od, struct > hmap *lflows) > > * Not to do conntrack on ND and ICMP destination > > * unreachable packets. */ > > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, > > - "nd || nd_rs || nd_ra || icmp4.type == 3 || " > > - "icmp6.type == 1 || " > > + "nd || nd_rs || nd_ra || " > > "(udp && udp.src == 546 && udp.dst == 547)", > "next;"); > > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, > > - "nd || nd_rs || nd_ra || icmp4.type == 3 || " > > - "icmp6.type == 1 || " > > + "nd || nd_rs || nd_ra || " > > "(udp && udp.src == 546 && udp.dst == 547)", > "next;"); > > > > /* Ingress and Egress Pre-ACL Table (Priority 100). > > @@ -4841,12 +4839,10 @@ build_pre_lb(struct ovn_datapath *od, struct > hmap *lflows, > > { > > /* Do not send ND packets to conntrack */ > > ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, > > - "nd || nd_rs || nd_ra || icmp4.type == 3 ||" > > - "icmp6.type == 1", > > + "nd || nd_rs || nd_ra", > > "next;"); > > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, > > - "nd || nd_rs || nd_ra || icmp4.type == 3 ||" > > - "icmp6.type == 1", > > + "nd || nd_rs || nd_ra", > > "next;"); > > > > /* Do not send service monitor packets to conntrack. */ > > @@ -5025,9 +5021,10 @@ build_reject_acl_rules(struct ovn_datapath *od, > struct hmap *lflows, > > ds_put_format(&actions, "%s ", extra_actions->string); > > } > > ds_put_format(&actions, "reg0 = 0; " > > - "eth.dst <-> eth.src; ip4.dst <-> ip4.src; " > > - "icmp4 { outport <-> inport; %s };", > > - ingress ? "output;" : > "next(pipeline=ingress,table=0);"); > > + "icmp4 { eth.dst <-> eth.src; ip4.dst <-> ip4.src; " > > + "outport <-> inport; %s };", > > + ingress ? "next(pipeline=egress,table=5);" > > + : "next(pipeline=ingress,table=19);"); > > ovn_lflow_add_with_hint(lflows, od, stage, > > acl->priority + OVN_ACL_PRI_OFFSET, > > ds_cstr(&match), ds_cstr(&actions), > stage_hint); > > @@ -5044,7 +5041,8 @@ build_reject_acl_rules(struct ovn_datapath *od, > struct hmap *lflows, > > ds_put_format(&actions, "reg0 = 0; icmp6 { " > > "eth.dst <-> eth.src; ip6.dst <-> ip6.src; " > > "outport <-> inport; %s };", > > - ingress ? "output;" : > "next(pipeline=ingress,table=0);"); > > + ingress ? "next(pipeline=egress,table=5);" > > + : "next(pipeline=ingress,table=19);"); > > ovn_lflow_add_with_hint(lflows, od, stage, > > acl->priority + OVN_ACL_PRI_OFFSET, > > ds_cstr(&match), ds_cstr(&actions), > stage_hint); > > diff --git a/tests/ovn.at b/tests/ovn.at > > index e6febd4c2..6467bdc42 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -11813,13 +11813,13 @@ test_ip_packet() { > > > > local ip_ttl=ff > > local > packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > > - > > + local > orig_pkt_in_reply=4500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > > local reply_icmp_ttl=ff > > local icmp_type_code_response=0301 > > local icmp_data=00000000 > > local > reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} > > - local > reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload} > > - echo $reply >> vif$inport.expected > > + local > reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload} > > + echo $reply$orig_pkt_in_reply >> vif$inport.expected > > > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > > } > > @@ -11836,7 +11836,7 @@ test_ipv6_packet() { > > local ip6_hdr=6000000000083aff${ipv6_src}${ipv6_dst} > > local packet=${eth_dst}${eth_src}86dd${ip6_hdr}0000000000000000 > > > > - local > reply=${eth_src}${eth_dst}86dd6000000000303aff${ipv6_dst}${ipv6_src}0101${exp_icmp_chksum}00000000${ip6_hdr} > > + local > reply=${eth_src}${eth_dst}86dd6000000000383aff${ipv6_dst}${ipv6_src}0101${exp_icmp_chksum}00000000${ip6_hdr}0000000000000000 > > echo $reply >> vif$inport.expected > > > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > > @@ -11914,11 +11914,11 @@ ovn-nbctl --log acl-add sw0 from-lport 1000 > "inport == \"sw0-p21\"" reject > > # Allow some time for ovn-northd and ovn-controller to catch up. > > ovn-nbctl --timeout=3 --wait=hv sync > > > > -test_ip_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) > $(ip_to_hex 192 168 1 21) 0000 7d8d fcfe > > -test_ip_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) > $(ip_to_hex 192 168 1 11) 0000 7d8d fcfe > > -test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) > $(ip_to_hex 192 168 1 12) 0000 7d82 fcfe > > +test_ip_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) > $(ip_to_hex 192 168 1 21) 0000 f85b f576 > > +test_ip_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) > $(ip_to_hex 192 168 1 11) 0000 f85b f576 > > +test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) > $(ip_to_hex 192 168 1 12) 0000 f850 f56b > > > > -test_ipv6_packet 11 1 000000000011 000000000021 > fe80000000000000020001fffe000001 fe80000000000000020001fffe000002 6183 > > +test_ipv6_packet 11 1 000000000011 000000000021 > fe80000000000000020001fffe000001 fe80000000000000020001fffe000002 617b > > > > test_tcp_syn_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 > 1 11) $(ip_to_hex 192 168 1 21) 0000 8b40 3039 0000 b85f 70e4 > > 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 > > @@ -12871,13 +12871,13 @@ test_ip_packet() { > > > > local ip_ttl=01 > > local > packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > > - > > + local > orig_pkt_in_reply=4500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} > > local reply_icmp_ttl=fe > > local icmp_type_code_response=0b00 > > local icmp_data=00000000 > > local > reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} > > - local > reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > > - echo $reply >> vif$inport.expected > > + local > reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > > + echo $reply$orig_pkt_in_reply >> vif$inport.expected > > > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > > } > > @@ -12895,7 +12895,7 @@ test_ip6_packet() { > > local ip6_hdr=6000000000151101${ipv6_src}${ipv6_dst} > > local > packet=${eth_dst}${eth_src}86dd${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a > > > > - local > reply=${eth_src}${eth_dst}86dd6000000000303afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr} > > + local > reply=${eth_src}${eth_dst}86dd6000000000453afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a > > echo $reply >> vif$inport.expected > > > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > > @@ -12937,8 +12937,8 @@ OVN_POPULATE_ARP > > # allow some time for ovn-northd and ovn-controller to catch up. > > ovn-nbctl --wait=hv sync > > > > -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) > $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 7dae f4ff > > -test_ip6_packet 1 1 000000000001 00000000ff01 > 20010db8000100000000000000000011 20010db8000200000000000000000011 > 20010db8000100000000000000000001 d461 > > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) > $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 f87c ea96 > > +test_ip6_packet 1 1 000000000001 00000000ff01 > 20010db8000100000000000000000011 20010db8000200000000000000000011 > 20010db8000100000000000000000001 1c22 > > OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) > > > > OVN_CLEANUP([hv1], [hv2]) > > @@ -12967,12 +12967,12 @@ test_ip_packet() { > > > > local ip_ttl=ff > > local > packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}${l4_proto}${ip_chksum}${ipv4_src}${ip_router} > > - > > + local > orig_pkt_in_reply=4500001400004000${ip_ttl}${l4_proto}${ip_chksum}${ipv4_src}${ip_router} > > local reply_icmp_ttl=fe > > local icmp_data=00000000 > > local > reply_icmp_payload=${exp_icmp_code}${exp_icmp_chksum}${icmp_data} > > - local > reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > > - echo $reply >> vif$inport.expected > > + local > reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > > + echo $reply$orig_pkt_in_reply >> vif$inport.expected > > > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > > } > > @@ -13038,7 +13038,9 @@ test_ip6_packet() { > > local > ip6_hdr=60000000${ipv6_len}${ipv6_proto}ff${ipv6_src}${ipv6_dst} > > local packet=${eth_dst}${eth_src}86dd${ip6_hdr}${data} > > > > - local > reply=${eth_src}${eth_dst}86dd6000000000303afe${ipv6_dst}${ipv6_src}${exp_icmp_code}${exp_icmp_chksum}00000000${ip6_hdr} > > + local reply_ip_len=`expr 48 + ${#data} / 2` > > + reply_ip_len=$(printf "%x" $reply_ip_len) > > + local > reply=${eth_src}${eth_dst}86dd6000000000${reply_ip_len}3afe${ipv6_dst}${ipv6_src}${exp_icmp_code}${exp_icmp_chksum}00000000${ip6_hdr}${data} > > echo $reply >> vif$inport.expected > > > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > > @@ -13080,13 +13082,13 @@ OVN_POPULATE_ARP > > # allow some time for ovn-northd and ovn-controller to catch up. > > ovn-nbctl --wait=hv sync > > > > -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) > $(ip_to_hex 192 168 1 254) 11 0000 7dae fcfc 0303 > > -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) > $(ip_to_hex 192 168 1 254) 84 0000 7dae fcfd 0302 > > -test_ip6_packet 1 1 000000000001 00000000ff01 > 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015 > dbb8303900155bac6b646f65206676676e6d66720a 0104 d570 > > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) > $(ip_to_hex 192 168 1 254) 11 0000 f87c f485 0303 > > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) > $(ip_to_hex 192 168 1 254) 84 0000 f87c f413 0302 > > +test_ip6_packet 1 1 000000000001 00000000ff01 > 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015 > dbb8303900155bac6b646f65206676676e6d66720a 0104 1d31 > > OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) > > > > test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2 > 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 b680 6e05 > > -test_ip6_packet 2 2 000000000002 00000000ff02 > 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004 > 01020304 0103 627e > > +test_ip6_packet 2 2 000000000002 00000000ff02 > 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004 > 01020304 0103 5e74 > > test_tcp6_packet 2 2 000000000002 00000000ff02 > 20010db8000200000000000000000011 20010db8000200000000000000000001 8b40 3039 > 0000 98cd > > OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [vif2.expected]) > > > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index fa3b83cb1..117f1e835 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -3697,7 +3697,7 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port > patch-.*/d > > AT_CLEANUP > > > > > > -AT_SETUP([ovn -- ACL reject - TCP reset]) > > +AT_SETUP([ovn -- ACL reject]) > > AT_SKIP_IF([test $HAVE_NC = no]) > > AT_KEYWORDS([lb]) > > > > @@ -3736,13 +3736,14 @@ ovn-nbctl acl-add pg0_drop from-lport 1001 > "inport == @pg0_drop && ip" drop > > ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" > drop > > > > ovn-nbctl pg-add pg0 sw0-p1-rej sw0-p2-rej > > -ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" > allow-related > > +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip" > allow-related > > ovn-nbctl --log acl-add pg0 from-lport 1004 "inport == @pg0 && ip && > tcp && tcp.dst == 80" reject > > +ovn-nbctl --log acl-add pg0 from-lport 1004 "inport == @pg0 && ip && > udp && udp.dst == 90" reject > > > > -ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && icmp4" allow-related > > ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && tcp && tcp.dst == 82" allow-related > > ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src > == 0.0.0.0/0 && udp && udp.dst == 82" allow-related > > ovn-nbctl --log acl-add pg0 to-lport 1004 "inport == @pg0 && ip && tcp > && tcp.dst == 84" reject > > +ovn-nbctl --log acl-add pg0 to-lport 1004 "inport == @pg0 && ip && udp > && udp.dst == 94" reject > > > > OVN_POPULATE_ARP > > ovn-nbctl --wait=hv sync > > @@ -3758,33 +3759,38 @@ ADD_VETH(sw0-p2-rej, sw0-p2-rej, br-int, " > 10.0.0.4/24", "50:54:00:00:00:04", \ > > NS_CHECK_EXEC([sw0-p1-rej], [ip a a aef0::3/64 dev sw0-p1-rej], [0]) > > NS_CHECK_EXEC([sw0-p2-rej], [ip a a aef0::4/64 dev sw0-p2-rej], [0]) > > > > -# Capture packets in sw0-p1-rej. > > -NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 2 -i sw0-p1-rej tcp port 80 > > sw0-p1-rej-ip4.pcap &], [0]) > > sleep 1 > > > > -NS_CHECK_EXEC([sw0-p1-rej], [nc 10.0.0.4 80], [1], [], > > -[dnl > > -Ncat: Connection refused. > > -]) > > +# Capture packets in sw0-p1-rej. > > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 4 -i sw0-p1-rej tcp > > sw0-p1-rej-ip4.pcap &], [0]) > > + > > +sleep 1 > > > > OVS_WAIT_UNTIL([ > > - total=`cat sw0-p1-rej-ip4.pcap | wc -l` > > - echo "total = $total" > > - test "${total}" = "2" > > + ip netns exec sw0-p1-rej nc 10.0.0.4 80 2> r > > + res=$(cat r) > > + test "$res" = "Ncat: Connection refused." > > ]) > > > > # Now send traffic to port 84 > > -NS_CHECK_EXEC([sw0-p1-rej], [nc 10.0.0.4 84], [1], [], > > -[dnl > > -Ncat: Connection refused. > > +OVS_WAIT_UNTIL([ > > + ip netns exec sw0-p1-rej nc 10.0.0.4 84 2> r > > + res=$(cat r) > > + test "$res" = "Ncat: Connection refused." > > ]) > > > > -AT_CHECK([ > > +OVS_WAIT_UNTIL([ > > n_pkt=$(ovs-ofctl dump-flows br-int table=44 | grep -v n_packets=0 > | \ > > grep controller | grep tp_dst=84 -c) > > test $n_pkt -eq 1 > > ]) > > > > +OVS_WAIT_UNTIL([ > > + total=`cat sw0-p1-rej-ip4.pcap | wc -l` > > + echo "total = $total" > > + test "${total}" = "4" > > +]) > > + > > # Without this sleep, test case fails intermittently. > > sleep 3 > > > > @@ -3792,17 +3798,68 @@ NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 2 -i > sw0-p2-rej tcp port 80 > sw0-p2- > > > > sleep 1 > > > > -NS_CHECK_EXEC([sw0-p2-rej], [nc -6 aef0::3 80], [1], [], > > -[dnl > > -Ncat: Connection refused. > > +OVS_WAIT_UNTIL([ > > + ip netns exec sw0-p2-rej nc -6 aef0::3 80 2> r > > + res=$(cat r) > > + test "$res" = "Ncat: Connection refused." > > ]) > > > > + > > OVS_WAIT_UNTIL([ > > total=`cat sw0-p2-rej-ip6.pcap | wc -l` > > echo "total = $total" > > test "${total}" = "2" > > ]) > > > > +# Now test for IPv4 UDP. > > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 90 > > sw0-p1-rej-udp.pcap &], [0]) > > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp > > sw0-p1-rej-icmp.pcap &], [0]) > > + > > +echo "foo" > foo > > +OVS_WAIT_UNTIL([ > > + ip netns exec sw0-p1-rej nc -u 10.0.0.4 90 < foo > > + c=$(cat sw0-p1-rej-icmp.pcap | grep \ > > +"10.0.0.4 > 10.0.0.3: ICMP 10.0.0.4 udp port dnsix unreachable" | uniq > | wc -l) > > + test $c -eq 1 > > +]) > > + > > +rm -f *.pcap > > + > > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 94 > > sw0-p1-rej-udp.pcap &], [0]) > > +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp > > sw0-p1-rej-icmp.pcap &], [0]) > > + > > +OVS_WAIT_UNTIL([ > > + ip netns exec sw0-p1-rej nc -u 10.0.0.4 94 < foo > > + c=$(cat sw0-p1-rej-icmp.pcap | grep \ > > +"10.0.0.4 > 10.0.0.3: ICMP 10.0.0.4 udp port objcall unreachable" | > uniq | wc -l) > > + test $c -eq 1 > > +]) > > + > > +# Now test for IPv6 UDP. > > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej udp port 90 > > sw0-p2-rej-ip6-udp.pcap &], [0]) > > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej icmp6 > > sw0-p2-rej-icmp6.pcap &], [0]) > > + > > +OVS_WAIT_UNTIL([ > > + ip netns exec sw0-p2-rej nc -u -6 aef0::3 90 < foo > > + c=$(cat sw0-p2-rej-icmp6.pcap | grep \ > > +"IP6 aef0::3 > aef0::4: ICMP6, destination unreachable, unreachable > port, \ > > +aef0::3 udp port dnsix" | uniq | wc -l) > > + test $c -eq 1 > > +]) > > + > > +rm -f *.pcap > > + > > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej udp port 94 > > sw0-p2-rej-ip6-udp.pcap &], [0]) > > +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej icmp6 > > sw0-p2-rej-icmp6.pcap &], [0]) > > + > > +OVS_WAIT_UNTIL([ > > + ip netns exec sw0-p2-rej nc -u -6 aef0::3 94 < foo > > + c=$(cat sw0-p2-rej-icmp6.pcap | grep \ > > +"IP6 aef0::3 > aef0::4: ICMP6, destination unreachable, unreachable > port, \ > > +aef0::3 udp port objcall" | uniq | wc -l) > > + test $c -eq 1 > > +]) > > + > > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > > > as ovn-sb > > -- > > 2.25.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 3230bb386..63796d88c 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -1453,7 +1453,7 @@ static void pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, struct dp_packet *pkt_in, const struct match *md, struct ofpbuf *userdata, - bool include_orig_ip_datagram) + bool set_icmp_code) { /* 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. */ @@ -1500,46 +1500,51 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, packet_set_ipv4(&packet, ip_flow->nw_src, ip_flow->nw_dst, ip_flow->nw_tos, 255); + uint8_t icmp_code = 1; + if (set_icmp_code && in_ip->ip_proto == IPPROTO_UDP) { + icmp_code = 3; + } + struct icmp_header *ih = dp_packet_put_zeros(&packet, sizeof *ih); dp_packet_set_l4(&packet, ih); - packet_set_icmp(&packet, ICMP4_DST_UNREACH, 1); - - if (include_orig_ip_datagram) { - /* RFC 1122: 3.2.2 MUST send at least the IP header and 8 bytes - * of header. MAY send more. - * RFC says return as much as we can without exceeding 576 - * bytes. - * So, lets return as much as we can. */ - - /* Calculate available room to include the original IP + data. */ - nh = dp_packet_l3(&packet); - uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len)); - if (in_ip_len > room) { - in_ip_len = room; - } - dp_packet_put(&packet, in_ip, in_ip_len); - - /* dp_packet_put may reallocate the buffer. Get the l3 and l4 - * header pointers again. */ - nh = dp_packet_l3(&packet); - ih = dp_packet_l4(&packet); - uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len; - nh->ip_tot_len = htons(ip_total_len); - ih->icmp_csum = 0; - ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len); - nh->ip_csum = 0; - nh->ip_csum = csum(nh, sizeof *nh); - } + packet_set_icmp(&packet, ICMP4_DST_UNREACH, icmp_code); + + /* RFC 1122: 3.2.2 MUST send at least the IP header and 8 bytes + * of header. MAY send more. + * RFC says return as much as we can without exceeding 576 + * bytes. + * So, lets return as much as we can. */ + + /* Calculate available room to include the original IP + data. */ + nh = dp_packet_l3(&packet); + uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len)); + if (in_ip_len > room) { + in_ip_len = room; + } + dp_packet_put(&packet, in_ip, in_ip_len); + + /* dp_packet_put may reallocate the buffer. Get the l3 and l4 + * header pointers again. */ + nh = dp_packet_l3(&packet); + ih = dp_packet_l4(&packet); + uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len; + nh->ip_tot_len = htons(ip_total_len); + ih->icmp_csum = 0; + ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len); + nh->ip_csum = 0; + nh->ip_csum = csum(nh, sizeof *nh); + } else { struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh); struct icmp6_data_header *ih; uint32_t icmpv6_csum; + struct ip6_hdr *in_ip = dp_packet_l3(pkt_in); eh->eth_type = htons(ETH_TYPE_IPV6); dp_packet_set_l3(&packet, nh); nh->ip6_vfc = 0x60; nh->ip6_nxt = IPPROTO_ICMPV6; - nh->ip6_plen = htons(sizeof(*nh) + ICMP6_DATA_HEADER_LEN); + nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN); packet_set_ipv6(&packet, &ip_flow->ipv6_src, &ip_flow->ipv6_dst, ip_flow->nw_tos, ip_flow->ipv6_label, 255); @@ -1547,15 +1552,42 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, dp_packet_set_l4(&packet, ih); ih->icmp6_base.icmp6_type = ICMP6_DST_UNREACH; ih->icmp6_base.icmp6_code = 1; + + if (set_icmp_code && in_ip->ip6_nxt == IPPROTO_UDP) { + ih->icmp6_base.icmp6_code = ICMP6_DST_UNREACH_NOPORT; + } ih->icmp6_base.icmp6_cksum = 0; - uint8_t *data = dp_packet_put_zeros(&packet, sizeof *nh); - memcpy(data, dp_packet_l3(pkt_in), sizeof(*nh)); + nh = dp_packet_l3(&packet); + + /* RFC 4443: 3.1. + * + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * | Type | Code | Checksum | + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * | Unused | + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * | As much of invoking packet | + * + as possible without the ICMPv6 packet + + * | exceeding the minimum IPv6 MTU [IPv6] | + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + */ + + uint16_t room = 1280 - (sizeof *eh + sizeof *nh + + ICMP6_DATA_HEADER_LEN); + uint16_t in_ip_len = (uint16_t) sizeof *in_ip + ntohs(in_ip->ip6_plen); + if (in_ip_len > room) { + in_ip_len = room; + } + + dp_packet_put(&packet, in_ip, in_ip_len); + nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN + in_ip_len); icmpv6_csum = packet_csum_pseudoheader6(dp_packet_l3(&packet)); ih->icmp6_base.icmp6_cksum = csum_finish( csum_continue(icmpv6_csum, ih, - sizeof(*nh) + ICMP6_DATA_HEADER_LEN)); + in_ip_len + ICMP6_DATA_HEADER_LEN)); } if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) { @@ -2646,12 +2678,12 @@ process_packet_in(struct rconn *swconn, const struct ofp_header *msg) case ACTION_OPCODE_ICMP: pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, - &userdata, false); + &userdata, true); break; case ACTION_OPCODE_ICMP4_ERROR: pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, - &userdata, true); + &userdata, false); break; case ACTION_OPCODE_TCP_RESET: diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 63753ac61..eb459c8c4 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -4724,12 +4724,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) * Not to do conntrack on ND and ICMP destination * unreachable packets. */ ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, - "nd || nd_rs || nd_ra || icmp4.type == 3 || " - "icmp6.type == 1 || " + "nd || nd_rs || nd_ra || " "(udp && udp.src == 546 && udp.dst == 547)", "next;"); ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, - "nd || nd_rs || nd_ra || icmp4.type == 3 || " - "icmp6.type == 1 || " + "nd || nd_rs || nd_ra || " "(udp && udp.src == 546 && udp.dst == 547)", "next;"); /* Ingress and Egress Pre-ACL Table (Priority 100). @@ -4841,12 +4839,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, { /* Do not send ND packets to conntrack */ ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, - "nd || nd_rs || nd_ra || icmp4.type == 3 ||" - "icmp6.type == 1", + "nd || nd_rs || nd_ra", "next;"); ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, - "nd || nd_rs || nd_ra || icmp4.type == 3 ||" - "icmp6.type == 1", + "nd || nd_rs || nd_ra", "next;"); /* Do not send service monitor packets to conntrack. */ @@ -5025,9 +5021,10 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, ds_put_format(&actions, "%s ", extra_actions->string); } ds_put_format(&actions, "reg0 = 0; " - "eth.dst <-> eth.src; ip4.dst <-> ip4.src; " - "icmp4 { outport <-> inport; %s };", - ingress ? "output;" : "next(pipeline=ingress,table=0);"); + "icmp4 { eth.dst <-> eth.src; ip4.dst <-> ip4.src; " + "outport <-> inport; %s };", + ingress ? "next(pipeline=egress,table=5);" + : "next(pipeline=ingress,table=19);"); ovn_lflow_add_with_hint(lflows, od, stage, acl->priority + OVN_ACL_PRI_OFFSET, ds_cstr(&match), ds_cstr(&actions), stage_hint); @@ -5044,7 +5041,8 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, ds_put_format(&actions, "reg0 = 0; icmp6 { " "eth.dst <-> eth.src; ip6.dst <-> ip6.src; " "outport <-> inport; %s };", - ingress ? "output;" : "next(pipeline=ingress,table=0);"); + ingress ? "next(pipeline=egress,table=5);" + : "next(pipeline=ingress,table=19);"); ovn_lflow_add_with_hint(lflows, od, stage, acl->priority + OVN_ACL_PRI_OFFSET, ds_cstr(&match), ds_cstr(&actions), stage_hint); diff --git a/tests/ovn.at b/tests/ovn.at index e6febd4c2..6467bdc42 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -11813,13 +11813,13 @@ test_ip_packet() { local ip_ttl=ff local packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} - + local orig_pkt_in_reply=4500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} local reply_icmp_ttl=ff local icmp_type_code_response=0301 local icmp_data=00000000 local reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} - local reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload} - echo $reply >> vif$inport.expected + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload} + echo $reply$orig_pkt_in_reply >> vif$inport.expected as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet } @@ -11836,7 +11836,7 @@ test_ipv6_packet() { local ip6_hdr=6000000000083aff${ipv6_src}${ipv6_dst} local packet=${eth_dst}${eth_src}86dd${ip6_hdr}0000000000000000 - local reply=${eth_src}${eth_dst}86dd6000000000303aff${ipv6_dst}${ipv6_src}0101${exp_icmp_chksum}00000000${ip6_hdr} + local reply=${eth_src}${eth_dst}86dd6000000000383aff${ipv6_dst}${ipv6_src}0101${exp_icmp_chksum}00000000${ip6_hdr}0000000000000000 echo $reply >> vif$inport.expected as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet @@ -11914,11 +11914,11 @@ ovn-nbctl --log acl-add sw0 from-lport 1000 "inport == \"sw0-p21\"" reject # Allow some time for ovn-northd and ovn-controller to catch up. ovn-nbctl --timeout=3 --wait=hv sync -test_ip_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 7d8d fcfe -test_ip_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 7d8d fcfe -test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 7d82 fcfe +test_ip_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 f85b f576 +test_ip_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 f85b f576 +test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 f850 f56b -test_ipv6_packet 11 1 000000000011 000000000021 fe80000000000000020001fffe000001 fe80000000000000020001fffe000002 6183 +test_ipv6_packet 11 1 000000000011 000000000021 fe80000000000000020001fffe000001 fe80000000000000020001fffe000002 617b test_tcp_syn_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 8b40 3039 0000 b85f 70e4 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 @@ -12871,13 +12871,13 @@ test_ip_packet() { local ip_ttl=01 local packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} - + local orig_pkt_in_reply=4500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst} local reply_icmp_ttl=fe local icmp_type_code_response=0b00 local icmp_data=00000000 local reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} - local reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} - echo $reply >> vif$inport.expected + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} + echo $reply$orig_pkt_in_reply >> vif$inport.expected as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet } @@ -12895,7 +12895,7 @@ test_ip6_packet() { local ip6_hdr=6000000000151101${ipv6_src}${ipv6_dst} local packet=${eth_dst}${eth_src}86dd${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a - local reply=${eth_src}${eth_dst}86dd6000000000303afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr} + local reply=${eth_src}${eth_dst}86dd6000000000453afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a echo $reply >> vif$inport.expected as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet @@ -12937,8 +12937,8 @@ OVN_POPULATE_ARP # allow some time for ovn-northd and ovn-controller to catch up. ovn-nbctl --wait=hv sync -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 7dae f4ff -test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 d461 +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 f87c ea96 +test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 1c22 OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) OVN_CLEANUP([hv1], [hv2]) @@ -12967,12 +12967,12 @@ test_ip_packet() { local ip_ttl=ff local packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}${l4_proto}${ip_chksum}${ipv4_src}${ip_router} - + local orig_pkt_in_reply=4500001400004000${ip_ttl}${l4_proto}${ip_chksum}${ipv4_src}${ip_router} local reply_icmp_ttl=fe local icmp_data=00000000 local reply_icmp_payload=${exp_icmp_code}${exp_icmp_chksum}${icmp_data} - local reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} - echo $reply >> vif$inport.expected + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} + echo $reply$orig_pkt_in_reply >> vif$inport.expected as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet } @@ -13038,7 +13038,9 @@ test_ip6_packet() { local ip6_hdr=60000000${ipv6_len}${ipv6_proto}ff${ipv6_src}${ipv6_dst} local packet=${eth_dst}${eth_src}86dd${ip6_hdr}${data} - local reply=${eth_src}${eth_dst}86dd6000000000303afe${ipv6_dst}${ipv6_src}${exp_icmp_code}${exp_icmp_chksum}00000000${ip6_hdr} + local reply_ip_len=`expr 48 + ${#data} / 2` + reply_ip_len=$(printf "%x" $reply_ip_len) + local reply=${eth_src}${eth_dst}86dd6000000000${reply_ip_len}3afe${ipv6_dst}${ipv6_src}${exp_icmp_code}${exp_icmp_chksum}00000000${ip6_hdr}${data} echo $reply >> vif$inport.expected as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet @@ -13080,13 +13082,13 @@ OVN_POPULATE_ARP # allow some time for ovn-northd and ovn-controller to catch up. ovn-nbctl --wait=hv sync -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 11 0000 7dae fcfc 0303 -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 84 0000 7dae fcfd 0302 -test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015 dbb8303900155bac6b646f65206676676e6d66720a 0104 d570 +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 11 0000 f87c f485 0303 +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 1 254) 84 0000 f87c f413 0302 +test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015 dbb8303900155bac6b646f65206676676e6d66720a 0104 1d31 OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 b680 6e05 -test_ip6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004 01020304 0103 627e +test_ip6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004 01020304 0103 5e74 test_tcp6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 8b40 3039 0000 98cd OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [vif2.expected]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index fa3b83cb1..117f1e835 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -3697,7 +3697,7 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d AT_CLEANUP -AT_SETUP([ovn -- ACL reject - TCP reset]) +AT_SETUP([ovn -- ACL reject]) AT_SKIP_IF([test $HAVE_NC = no]) AT_KEYWORDS([lb]) @@ -3736,13 +3736,14 @@ ovn-nbctl acl-add pg0_drop from-lport 1001 "inport == @pg0_drop && ip" drop ovn-nbctl acl-add pg0_drop to-lport 1001 "outport == @pg0_drop && ip" drop ovn-nbctl pg-add pg0 sw0-p1-rej sw0-p2-rej -ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4" allow-related +ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip" allow-related ovn-nbctl --log acl-add pg0 from-lport 1004 "inport == @pg0 && ip && tcp && tcp.dst == 80" reject +ovn-nbctl --log acl-add pg0 from-lport 1004 "inport == @pg0 && ip && udp && udp.dst == 90" reject -ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && icmp4" allow-related ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && tcp && tcp.dst == 82" allow-related ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && ip4.src == 0.0.0.0/0 && udp && udp.dst == 82" allow-related ovn-nbctl --log acl-add pg0 to-lport 1004 "inport == @pg0 && ip && tcp && tcp.dst == 84" reject +ovn-nbctl --log acl-add pg0 to-lport 1004 "inport == @pg0 && ip && udp && udp.dst == 94" reject OVN_POPULATE_ARP ovn-nbctl --wait=hv sync @@ -3758,33 +3759,38 @@ ADD_VETH(sw0-p2-rej, sw0-p2-rej, br-int, "10.0.0.4/24", "50:54:00:00:00:04", \ NS_CHECK_EXEC([sw0-p1-rej], [ip a a aef0::3/64 dev sw0-p1-rej], [0]) NS_CHECK_EXEC([sw0-p2-rej], [ip a a aef0::4/64 dev sw0-p2-rej], [0]) -# Capture packets in sw0-p1-rej. -NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 2 -i sw0-p1-rej tcp port 80 > sw0-p1-rej-ip4.pcap &], [0]) sleep 1 -NS_CHECK_EXEC([sw0-p1-rej], [nc 10.0.0.4 80], [1], [], -[dnl -Ncat: Connection refused. -]) +# Capture packets in sw0-p1-rej. +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 4 -i sw0-p1-rej tcp > sw0-p1-rej-ip4.pcap &], [0]) + +sleep 1 OVS_WAIT_UNTIL([ - total=`cat sw0-p1-rej-ip4.pcap | wc -l` - echo "total = $total" - test "${total}" = "2" + ip netns exec sw0-p1-rej nc 10.0.0.4 80 2> r + res=$(cat r) + test "$res" = "Ncat: Connection refused." ]) # Now send traffic to port 84 -NS_CHECK_EXEC([sw0-p1-rej], [nc 10.0.0.4 84], [1], [], -[dnl -Ncat: Connection refused. +OVS_WAIT_UNTIL([ + ip netns exec sw0-p1-rej nc 10.0.0.4 84 2> r + res=$(cat r) + test "$res" = "Ncat: Connection refused." ]) -AT_CHECK([ +OVS_WAIT_UNTIL([ n_pkt=$(ovs-ofctl dump-flows br-int table=44 | grep -v n_packets=0 | \ grep controller | grep tp_dst=84 -c) test $n_pkt -eq 1 ]) +OVS_WAIT_UNTIL([ + total=`cat sw0-p1-rej-ip4.pcap | wc -l` + echo "total = $total" + test "${total}" = "4" +]) + # Without this sleep, test case fails intermittently. sleep 3 @@ -3792,17 +3798,68 @@ NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 2 -i sw0-p2-rej tcp port 80 > sw0-p2- sleep 1 -NS_CHECK_EXEC([sw0-p2-rej], [nc -6 aef0::3 80], [1], [], -[dnl -Ncat: Connection refused. +OVS_WAIT_UNTIL([ + ip netns exec sw0-p2-rej nc -6 aef0::3 80 2> r + res=$(cat r) + test "$res" = "Ncat: Connection refused." ]) + OVS_WAIT_UNTIL([ total=`cat sw0-p2-rej-ip6.pcap | wc -l` echo "total = $total" test "${total}" = "2" ]) +# Now test for IPv4 UDP. +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 90 > sw0-p1-rej-udp.pcap &], [0]) +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp > sw0-p1-rej-icmp.pcap &], [0]) + +echo "foo" > foo +OVS_WAIT_UNTIL([ + ip netns exec sw0-p1-rej nc -u 10.0.0.4 90 < foo + c=$(cat sw0-p1-rej-icmp.pcap | grep \ +"10.0.0.4 > 10.0.0.3: ICMP 10.0.0.4 udp port dnsix unreachable" | uniq | wc -l) + test $c -eq 1 +]) + +rm -f *.pcap + +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 94 > sw0-p1-rej-udp.pcap &], [0]) +NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp > sw0-p1-rej-icmp.pcap &], [0]) + +OVS_WAIT_UNTIL([ + ip netns exec sw0-p1-rej nc -u 10.0.0.4 94 < foo + c=$(cat sw0-p1-rej-icmp.pcap | grep \ +"10.0.0.4 > 10.0.0.3: ICMP 10.0.0.4 udp port objcall unreachable" | uniq | wc -l) + test $c -eq 1 +]) + +# Now test for IPv6 UDP. +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej udp port 90 > sw0-p2-rej-ip6-udp.pcap &], [0]) +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej icmp6 > sw0-p2-rej-icmp6.pcap &], [0]) + +OVS_WAIT_UNTIL([ + ip netns exec sw0-p2-rej nc -u -6 aef0::3 90 < foo + c=$(cat sw0-p2-rej-icmp6.pcap | grep \ +"IP6 aef0::3 > aef0::4: ICMP6, destination unreachable, unreachable port, \ +aef0::3 udp port dnsix" | uniq | wc -l) + test $c -eq 1 +]) + +rm -f *.pcap + +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej udp port 94 > sw0-p2-rej-ip6-udp.pcap &], [0]) +NS_CHECK_EXEC([sw0-p2-rej], [tcpdump -n -c 1 -i sw0-p2-rej icmp6 > sw0-p2-rej-icmp6.pcap &], [0]) + +OVS_WAIT_UNTIL([ + ip netns exec sw0-p2-rej nc -u -6 aef0::3 94 < foo + c=$(cat sw0-p2-rej-icmp6.pcap | grep \ +"IP6 aef0::3 > aef0::4: ICMP6, destination unreachable, unreachable port, \ +aef0::3 udp port objcall" | uniq | wc -l) + test $c -eq 1 +]) + OVS_APP_EXIT_AND_WAIT([ovn-controller]) as ovn-sb