Hi Han,

Thanks for your comments. I've decided to rewrite docs for this option 
taking your comments into account.

Please, see v3 here: 
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/

On 04.12.2024 05:20, Han Zhou wrote:
>
>
> On Thu, Nov 28, 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]>
> > ---
> > v1 -> v2:
> >   - replaced unordered with ordered list
> >   - removed src-ip based routes description as suggested by Han.
>
> Thanks Vladislav for v2. Please see some more comment below.
>
> > ---
> >  ovn-nb.xml | 26 ++++++++++++++------------
> >  1 file changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 5114bbc2e..a41dc6c1c 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -3877,24 +3877,26 @@ 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
>
> Sorry that I didn't notice this in v1. It seems confusing here because 
> maximum prefix length only applies within the same route table (and 
> with directly connected routes) but not across route tables. I think 
> this needs to be emphasized to avoid confusion. For example:
>
> route_table A: prefix X/24, nexthop-1
> route_table B: prefix X/25, nexthop-2
>
> For packets enters LRP with options:route_table = A, they should go to 
> nexthop-1 according to the implementation, but according to the above 
> documentation it should go to nexthop-2 because X/25 has longer prefix 
> length than X/24, which is misleading.
>
> > +        routes with same prefix length the following lookup order 
> applies:
> >        </p>
> >
> > -      <ul>
> > +      <ol>
> >          <li>
> > -          1. First lookup among "global" routes: routes without
> > -          <code>route_table</code> value set and routes to directly 
> connected
> > -          networks.
> > +          Lookup among routes of directly connected networks (including
> > +          directly connected networks learned from other 
> availability zones
> > +          within same LR through OVN-IC).
> >          </li>
> >          <li>
> > -          2. Next lookup among routes with same 
> <code>route_table</code> value
> > -          as specified in LRP's options:route_table field.
> > +          Lookup among static routes with same <ref
> > +          table="Logical_Router_Static_Route" 
> column="route_table"/> value
> > +          as specified in <ref table="Logical_Router_Port" 
> column="options"
> > +          key="route_table"/> field.  If no value specified in <ref
> > +          table="Logical_Router_Port" column="options" 
> key="route_table"/>,
>
> Please note that the part (ref table="xxx") will not appear in the 
> text after the XML is converted to manpage. It needs to be something 
> like: Lookup among static routes with same <ref 
> table="Logical_Router_Static_Route" column="route_table"/> value of 
> the <ref table="Logical_Router_Static_Route"/> table as specified in 
> the <ref table="Logical_Router_Port" column="options" 
> key="route_table"/> field of the <ref table="Logical_Router_Port"/>.
>
> We should also mention that the LRP is where the packets enter the LR.
>
> Best,
> Han
>
> > +          static routes with empty <code>route_table</code> are 
> looked up.
> >          </li>
> > -      </ul>
> > +      </ol>
> >      </column>
> >
> >      <column name="external_ids" key="ic-learned-route">
> > --
> > 2.46.2

-- 
Regards,
Vladislav Odintsov

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to