[ovs-dev] [PATCH v2 ovn 2/3] northd: enable check_pkt_larger for gw router

Mark Michelson mmichels at redhat.com
Tue Jun 8 16:46:12 UTC 2021


Equivalent DDLog code is missing from this patch.

The new test is pretty much exactly the same as "ovn -- router - check 
packet length - icmp defrag". I think there are about 3-4 changed lines, 
including the title and keywords. I think you could add the gateway 
router testing to the existing test instead of writing an entirely new test.

On 5/27/21 6:26 PM, Lorenzo Bianconi wrote:
> As it is already done for distributed gw router scenario, introduce
> check_pkt_larger logical flows for gw router use case.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
>   northd/ovn-northd.c |  31 ++++--
>   tests/ovn.at        | 238 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 260 insertions(+), 9 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index d849e6abc..2269f0185 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -10525,17 +10525,30 @@ build_check_pkt_len_flows_for_lrouter(
>           struct hmap *ports,
>           struct ds *match, struct ds *actions)
>   {
> -    if (od->nbr) {
> +    if (!od->nbr) {
> +        return;
> +    }
>   
> -        /* Packets are allowed by default. */
> -        ovn_lflow_add(lflows, od, S_ROUTER_IN_CHK_PKT_LEN, 0, "1",
> -                      "next;");
> -        ovn_lflow_add(lflows, od, S_ROUTER_IN_LARGER_PKTS, 0, "1",
> -                      "next;");
> +    /* Packets are allowed by default. */
> +    ovn_lflow_add(lflows, od, S_ROUTER_IN_CHK_PKT_LEN, 0, "1",
> +                  "next;");
> +    ovn_lflow_add(lflows, od, S_ROUTER_IN_LARGER_PKTS, 0, "1",
> +                  "next;");
>   
> -        if (od->l3dgw_port && od->l3redirect_port) {
> -            build_check_pkt_len_flows_for_lrp(od->l3dgw_port, lflows,
> -                                              ports, match, actions);
> +    if (od->l3dgw_port && od->l3redirect_port) {
> +        /* gw router port */
> +        build_check_pkt_len_flows_for_lrp(od->l3dgw_port, lflows,
> +                                          ports, match, actions);
> +    } else if (smap_get(&od->nbr->options, "chassis")) {
> +        for (size_t i = 0; i < od->nbr->n_ports; i++) {
> +            /* gw router */
> +            struct ovn_port *rp = ovn_port_find(ports,
> +                                                od->nbr->ports[i]->name);
> +            if (!rp) {
> +                continue;
> +            }
> +            build_check_pkt_len_flows_for_lrp(rp, lflows, ports, match,
> +                                              actions);
>           }
>       }
>   }
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 71d2bab4d..7ad0dcb54 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -16412,6 +16412,244 @@ OVN_CLEANUP([hv1])
>   AT_CLEANUP
>   ])
>   
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn -- gw router - check packet length - icmp defrag])
> +AT_KEYWORDS([gwr-check_packet_length])
> +ovn_start
> +
> +ovn-nbctl ls-add sw0
> +ovn-nbctl lsp-add sw0 sw0-port1
> +ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3 1000::3"
> +
> +ovn-nbctl create Logical_Router name=lr0 options:chassis="hv1"
> +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::1/64
> +ovn-nbctl lsp-add sw0 sw0-lr0
> +ovn-nbctl lsp-set-type sw0-lr0 router
> +ovn-nbctl lsp-set-addresses sw0-lr0 router
> +ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
> +
> +ovn-nbctl ls-add public
> +ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24 2000::1/64
> +ovn-nbctl lsp-add public public-lr0
> +ovn-nbctl lsp-set-type public-lr0 router
> +ovn-nbctl lsp-set-addresses public-lr0 router
> +ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
> +
> +# localnet port
> +ovn-nbctl lsp-add public ln-public
> +ovn-nbctl lsp-set-type ln-public localnet
> +ovn-nbctl lsp-set-addresses ln-public unknown
> +ovn-nbctl lsp-set-options ln-public network_name=phys
> +
> +ovn-nbctl lr-nat-add lr0 snat 172.168.0.100 10.0.0.0/24
> +ovn-nbctl lr-nat-add lr0 snat 2000::1 1000::/64
> +
> +net_add n1
> +
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +ovs-vsctl -- add-port br-int hv1-vif1 -- \
> +    set interface hv1-vif1 external-ids:iface-id=sw0-port1 \
> +    options:tx_pcap=hv1/vif1-tx.pcap \
> +    options:rxq_pcap=hv1/vif1-rx.pcap \
> +    ofport-request=1
> +
> +reset_pcap_file() {
> +     local iface=$1
> +     local pcap_file=$2
> +     ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> + options:rxq_pcap=dummy-rx.pcap
> +     rm -f ${pcap_file}*.pcap
> +     ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
> + options:rxq_pcap=${pcap_file}-rx.pcap
> +}
> +
> +test_ip_packet_larger() {
> +    local mtu=$1
> +
> +    # Send ip packet from sw0-port1 to outside
> +    src_mac="505400000001" # sw-port1 mac
> +    dst_mac="00000000ff01" # sw0-lr0 mac (internal router leg)
> +    src_ip=`ip_to_hex 10 0 0 3`
> +    dst_ip=`ip_to_hex 172 168 0 3`
> +    # Set the packet length to 118.
> +    pkt_len=0076
> +    packet=${dst_mac}${src_mac}08004500${pkt_len}000000004001c3d9
> +    orig_packet_l3=${src_ip}${dst_ip}0304000000000000
> +    orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000
> +    orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000
> +    orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000
> +    orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000
> +    orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000
> +
> +    packet=${packet}${orig_packet_l3}
> +
> +    gw_ip_garp=ffffffffffff00002020121308060001080006040001000020201213aca80064000000000000aca80064
> +
> +    packet_bytes=$(expr ${#packet} / 2)
> +    mtu_needed=$(expr ${packet_bytes} - 18)
> +
> +    # If icmp_pmtu_reply_expected is 0, it means the packet is lesser than
> +    # the gateway mtu and should be delivered to the provider bridge via the
> +    # localnet port.
> +    # If icmp_pmtu_reply_expected is 1, it means the packet is larger than
> +    # the gateway mtu and ovn-controller should drop the packet and instead
> +    # generate ICMPv4  Destination Unreachable message with pmtu set to 100.
> +    if test $mtu -ge $mtu_needed; then
> +        # Packet to expect at br-phys.
> +        src_mac="000020201213"
> +        dst_mac="00000012af11"
> +        src_ip=`ip_to_hex 10 0 0 3`
> +        dst_ip=`ip_to_hex 172 168 0 3`
> +        expected=${dst_mac}${src_mac}08004500${pkt_len}000000003f01c4d9
> +        expected=${expected}${src_ip}${dst_ip}0304000000000000
> +        expected=${expected}000000000000000000000000000000000000
> +        expected=${expected}000000000000000000000000000000000000
> +        expected=${expected}000000000000000000000000000000000000
> +        expected=${expected}000000000000000000000000000000000000
> +        expected=${expected}000000000000000000000000000000000000
> +        echo $expected > br_phys_n1.expected
> +        echo $gw_ip_garp >> br_phys_n1.expected
> +    else
> +        src_ip=`ip_to_hex 10 0 0 1`
> +        dst_ip=`ip_to_hex 10 0 0 3`
> +        # pkt len should be 146 (28 (icmp packet) + 118 (orig ip + payload))
> +        reply_pkt_len=0092
> +        ip_csum=f993
> +        icmp_reply=${src_mac}${dst_mac}08004500${reply_pkt_len}00004000fe016867
> +        icmp_reply=${icmp_reply}${src_ip}${dst_ip}0304${ip_csum}0000$(printf "%04x" $mtu)
> +        icmp_reply=${icmp_reply}4500${pkt_len}000000003f01c4d9
> +        icmp_reply=${icmp_reply}${orig_packet_l3}
> +        echo $icmp_reply > hv1-vif1.expected
> +    fi
> +
> +    as hv1 reset_pcap_file br-phys_n1 hv1/br-phys_n1
> +    as hv1 reset_pcap_file hv1-vif1 hv1/vif1
> +
> +    # Send packet from sw0-port1 to outside
> +    check as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
> +
> +    if test $mtu -ge $mtu_needed; then
> +        OVN_CHECK_PACKETS([hv1/br-phys_n1-tx.pcap], [br_phys_n1.expected])
> +        $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap  > pkts
> +        # hv1/vif1-tx.pcap can receive the GARP packet generated by ovn-controller
> +        # for the gateway router port. So ignore this packet.
> +        cat pkts | grep -v $gw_ip_garp > packets
> +        AT_CHECK([cat packets], [0], [])
> +    else
> +        OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [hv1-vif1.expected])
> +        $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/br-phys_n1-tx.pcap  > \
> +        pkts
> +        # hv1/br-phys_n1-tx.pcap can receive the GARP packet generated by ovn-controller
> +        # for the gateway router port. So ignore this packet.
> +        cat pkts | grep -v $gw_ip_garp > packets
> +        AT_CHECK([cat packets], [0], [])
> +    fi
> +}
> +
> +test_ip6_packet_larger() {
> +    local mtu=$1
> +
> +    local eth_src=505400000001
> +    local eth_dst=00000000ff01
> +
> +    local ipv6_src=10000000000000000000000000000003
> +    local ipv6_dst=20000000000000000000000000000002
> +    local ipv6_rt=10000000000000000000000000000001
> +
> +    local payload=0000000000000000000000000000000000000000
> +    local payload=${payload}0000000000000000000000000000000000000000
> +    local payload=${payload}0000000000000000000000000000000000000000
> +    local payload=${payload}0000000000000000000000000000000000000000
> +
> +    local ip6_hdr=6000000000583aff${ipv6_src}${ipv6_dst}
> +    local packet=${eth_dst}${eth_src}86dd${ip6_hdr}8000ec7662f00001${payload}
> +
> +    as hv1 reset_pcap_file br-phys_n1 hv1/br-phys_n1
> +    as hv1 reset_pcap_file hv1-vif1 hv1/vif1
> +
> +    # Send packet from sw0-port1 to outside
> +    tcpdump_hex ">> sending packet:" $packet
> +    check as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
> +    AT_CHECK([as hv1 ovs-appctl ofproto/trace br-int in_port=hv1-vif1 $packet > trace-$mtu],
> +             [0], [ignore])
> +    AT_CAPTURE_FILE([trace-$mtu])
> +
> +    packet_bytes=$(expr ${#packet} / 2)
> +    mtu_needed=$(expr ${packet_bytes} - 18)
> +    if test $mtu -lt $mtu_needed; then
> +        # First construct the inner IPv6 packet.
> +        inner_ip6=6000000000583afe${ipv6_src}${ipv6_dst}
> +        inner_icmp6=8000000062f00001
> +        inner_icmp6_and_payload=$(icmp6_csum_inplace ${inner_icmp6}${payload} ${inner_ip6})
> +        inner_packet=${inner_ip6}${inner_icmp6_and_payload}
> +
> +        # Then the outer.
> +        outer_ip6=6000000000883afe${ipv6_rt}${ipv6_src}
> +        outer_icmp6_and_payload=$(icmp6_csum_inplace 020000000000$(printf "%04x" $mtu)${inner_packet} $outer_ip6)
> +        outer_packet=${outer_ip6}${outer_icmp6_and_payload}
> +
> +        icmp6_reply=${eth_src}${eth_dst}86dd${outer_packet}
> +
> +        echo
> +        tcpdump_hex ">> expecting reply packet" $icmp6_reply
> +
> +        # The "trace" above sends a second packets as a side effect.
> +        (echo $icmp6_reply; echo $icmp6_reply) > hv1-vif1.expected
> +
> +        OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [hv1-vif1.expected])
> +    fi
> +}
> +
> +wait_for_ports_up
> +ovn-nbctl --wait=hv sync
> +
> +ovn-nbctl show > nbdump
> +AT_CAPTURE_FILE([nbdump])
> +
> +ovn-sbctl show > sbdump
> +AT_CAPTURE_FILE([sbdump])
> +
> +ovn-sbctl dump-flows > sbflows
> +AT_CAPTURE_FILE([sbflows])
> +
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int  \
> +| grep "check_pkt_larger" | wc -l], [0], [[0
> +]])
> +dp_uuid=$(ovn-sbctl find datapath_binding | grep sw0 -B2 | grep _uuid | \
> +awk '{print $3}')
> +ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \
> +logical_port=lr0-public mac="00\:00\:00\:12\:af\:11"
> +
> +# Try different gateway mtus and send a 142-byte packet (corresponding
> +# to a 124-byte MTU).  If the MTU is less than 124, ovn-controller
> +# should send icmp host not reachable with pmtu set to $mtu.
> +for mtu in 100 500 118; do
> +    AS_BOX([testing mtu $mtu])
> +    check ovn-nbctl --wait=hv set logical_router_port lr0-public options:gateway_mtu=$mtu
> +    ovn-sbctl dump-flows > sbflows-$mtu
> +    AT_CAPTURE_FILE([sbflows-$mtu])
> +
> +    OVS_WAIT_FOR_OUTPUT([
> +        as hv1 ovs-ofctl dump-flows br-int > br-int-flows-$mtu
> +        AT_CAPTURE_FILE([br-int-flows-$mtu])
> +        grep "check_pkt_larger($(expr $mtu + 18))" br-int-flows-$mtu | wc -l], [0], [1
> +])
> +
> +    AS_BOX([testing mtu $mtu - IPv4])
> +    test_ip_packet_larger $mtu
> +
> +    AS_BOX([testing mtu $mtu - IPv6])
> +    test_ip6_packet_larger $mtu
> +done
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> +])
> +
>   OVN_FOR_EACH_NORTHD([
>   AT_SETUP([ovn -- IP packet buffering])
>   AT_KEYWORDS([ip-buffering])
> 



More information about the dev mailing list