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

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Tue Jun 8 17:01:42 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.

ack, I will fix it in v3.

Regards,
Lorenzo

> 
> 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