Skip to content

Commit

Permalink
ovn-northd: Add flow to use eth.src if nd.tll is 0 in put_nd() action.
Browse files Browse the repository at this point in the history
Presently OVN assumes that an IPv6 Neigh Adv packet will have the
target link layer option present.  But that's not the case all the time.
This field is optional and as per rfc4861 (quoted below).

Target link-layer address
                     The link-layer address for the target, i.e., the
                     sender of the advertisement.  This option MUST be
                     included on link layers that have addresses when
                     responding to multicast solicitations.  When
                     responding to a unicast Neighbor Solicitation this
                     option SHOULD be included.

                     The option MUST be included for multicast
                     solicitations in order to avoid infinite Neighbor
                     Solicitation "recursion" when the peer node does
                     not have a cache entry to return a Neighbor
                     Advertisements message.  When responding to unicast
                     solicitations, the option can be omitted since the
                     sender of the solicitation has the correct link-
                     layer address; otherwise, it would not be able to
                     send the unicast solicitation in the first place.
                     However, including the link-layer address in this
                     case adds little overhead and eliminates a
                     potential race condition where the sender deletes
                     the cached link-layer address prior to receiving a
                     response to a previous solicitation.

If target link layer option is not present, then ovn-controller learns
the mac binding with 00:00:00:00:00:00 address which is not correct.

This patch fixes the issue by adding the below logical flow in router
pipeline:

table=2 (lr_in_learn_neighbor), priority=95   , match=(nd_na && nd.tll == 0),
         action=(put_nd(inport, nd.target, eth.src); next;)

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2078026
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 80187a8)
  • Loading branch information
numansiddique committed Apr 25, 2022
1 parent 6508882 commit bd0ee4c
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 0 deletions.
6 changes: 6 additions & 0 deletions northd/northd.c
Expand Up @@ -10815,6 +10815,12 @@ build_neigh_learning_flows_for_lrouter(
copp_meter_get(COPP_ARP, od->nbr->copp,
meter_groups));

ovn_lflow_metered(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 95,
"nd_na && nd.tll == 0",
"put_nd(inport, nd.target, eth.src); next;",
copp_meter_get(COPP_ND_NA, od->nbr->copp,
meter_groups));

ovn_lflow_metered(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 90,
"nd_na", "put_nd(inport, nd.target, nd.tll); next;",
copp_meter_get(COPP_ND_NA, od->nbr->copp,
Expand Down
6 changes: 6 additions & 0 deletions northd/ovn-northd.8.xml
Expand Up @@ -2301,6 +2301,12 @@ next;
<code>put_arp(inport, arp.spa, arp.sha); next;</code>
</li>

<li>
A priority-95 flow with the match <code>nd_na &amp;&amp;
nd.tll == 0</code> and applies the action
<code>put_nd(inport, nd.target, eth.src); next;</code>
</li>

<li>
A priority-90 flow with the match <code>nd_na</code> and
applies the action
Expand Down
25 changes: 25 additions & 0 deletions tests/ovn-northd.at
Expand Up @@ -6433,3 +6433,28 @@ AT_CHECK([grep -e "ls_in_stateful" lsflows | sed 's/table=../table=??/' | sort],

AT_CLEANUP
])

OVN_FOR_EACH_NORTHD([
AT_SETUP([LR neighbor lookup and learning flows])
ovn_start

# Create logical routers
ovn-nbctl --wait=sb lr-add lr0

ovn-sbctl dump-flows lr0 > lrflows
AT_CAPTURE_FILE([lrflows])

AT_CHECK([cat lrflows | grep -e lr_in_lookup_neighbor -e lr_in_learn_neighbor | sort], [0], [dnl
table=1 (lr_in_lookup_neighbor), priority=0 , match=(1), action=(reg9[[2]] = 1; next;)
table=1 (lr_in_lookup_neighbor), priority=100 , match=(arp.op == 2), action=(reg9[[2]] = lookup_arp(inport, arp.spa, arp.sha); next;)
table=1 (lr_in_lookup_neighbor), priority=100 , match=(nd_na), action=(reg9[[2]] = lookup_nd(inport, nd.target, nd.tll); next;)
table=1 (lr_in_lookup_neighbor), priority=100 , match=(nd_ns), action=(reg9[[2]] = lookup_nd(inport, ip6.src, nd.sll); next;)
table=2 (lr_in_learn_neighbor), priority=100 , match=(reg9[[2]] == 1), action=(next;)
table=2 (lr_in_learn_neighbor), priority=90 , match=(arp), action=(put_arp(inport, arp.spa, arp.sha); next;)
table=2 (lr_in_learn_neighbor), priority=90 , match=(nd_na), action=(put_nd(inport, nd.target, nd.tll); next;)
table=2 (lr_in_learn_neighbor), priority=90 , match=(nd_ns), action=(put_nd(inport, ip6.src, nd.sll); next;)
table=2 (lr_in_learn_neighbor), priority=95 , match=(nd_na && nd.tll == 0), action=(put_nd(inport, nd.target, eth.src); next;)
])

AT_CLEANUP
])

0 comments on commit bd0ee4c

Please sign in to comment.