Hi Han, On 28.11.2024 02:02, Han Zhou wrote: > > Thanks Vladislav. Please see my comments below. > > On Tue, Nov 26, 2024 at 11:48 PM Vladislav Odintsov > <[email protected]> wrote: > > > > Prior to this patch documentation for routes lookup did not match > the actual > > behavior. > > > > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2024-November/418485.html > > Fixes: 1655a6c146ca ("northd, utils: support for RouteTables in LRs") > > Signed-off-by: Vladislav Odintsov <[email protected]> > > --- > > ovn-nb.xml | 30 ++++++++++++++++++++---------- > > 1 file changed, 20 insertions(+), 10 deletions(-) > > > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index 5114bbc2e..9dc68732d 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -3877,22 +3877,32 @@ or > > > > <column name="route_table"> > > <p> > > - Any string to place route to separate routing table. If > Logical Router > > - Port has configured value in <ref table="Logical_Router_Port" > > - column="options" key="route_table"/> other than empty > string, OVN > > - performs route lookup for all packets entering Logical > Router ingress > > - pipeline from this port in the following manner: > > + Any string to place route to separate routing table. > Maximum prefix > > + length matched route takes precedence over others, while > for the same > > + routes with same prefix length the following lookup order > applies: > > </p> > > > > <ul> > > <li> > > - 1. First lookup among "global" routes: routes without > > - <code>route_table</code> value set and routes to directly > connected > > - networks. > > + 1. Lookup among directly connected to LR networks (including > > nit: For the grammar, it may be revised as: Lookup among routes of > directly connected networks ... Ack. > > > + directly connected routes learned from other availability > zones > > + within same LR through OVN-IC). > > + </li> > > + </ul> > > + <ul> > > + <li> > > + 2. Lookup among static routes with same > <code>route_table</code> > > + value as specified in <ref table="Logical_Router_Port" > > + column="options" key="route_table"/> field. > > + </li> > > + <li> > > + If no value specified in <ref table="Logical_Router_Port" > > + column="options" key="route_table"/>, static routes with > empty > > + <code>route_table</code> are looked up. > > </li> > > <li> > > - 2. Next lookup among routes with same > <code>route_table</code> value > > - as specified in LRP's options:route_table field. > > + If route is source-based, it has lower priority than > > + destination-based route with same CIDR. > > I think the description of the source-based route here is confusing. > In the earlier summary it was mentioning: "for the same routes with > same prefix length the following lookup order applies". However, > source-based routes are NOT "the same routes" as the dest-based > routes, because they check on totally different fields of the IP > header. They don't even have to have the same CIDR. A typical example > may be: > > Route 1: A/24, nexthop X, dst-ip > Route 2: B/24, nexthop Y, src-ip > In this example, the two routes have the same prefix length but > totally different CIDR. When a packet with dst_ip == A/24 and src_ip > == B/24 comes, it matches both routes, but the route 1 will take > precedence, because it is dst based. If route 2 is B/25, it would win, > although it doesn't seem to make much sense for any real world use cases. > > So in my opinion they shouldn't even be considered together, which is > why the patch > (https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/) > > was proposed. > > But I understand that your motivation here is to document what is > currently implemented accurately. I would avoid mentioning the source > routing here because the problem doesn't belong to the "for the same > routes with same prefix length" precondition, and there are already > separate documentations about the behavior of the source-based > routing, to avoid redundancy in the document. Agree. Will fix in v2 and send out shortly. > > Best, > Han > > > </li> > > </ul> > > </column> > > -- > > 2.46.2
-- Regards, Vladislav Odintsov _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
