On 10/27/25 1:47 PM, Dima Chumak wrote:
> On 2025-09-14 10:09 PM, Ilya Maximets wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 8/20/25 4:34 PM, Dima Chumak via dev wrote:
>>> Introduce support for route lookup across several routing tables
>>> following a priority from highest to lowest.
>>>
>>> To properly prioritize route lookup order, the single router table in
>>> OVS is now split into three tables: local, main and default.
>>> Corresponding routing rules are created by default, where local table
>>> has the highest priority and main and default tables have two lowest
>>> priorities respectively. All non-standard tables can be added with a
>>> priority in between local and main.
>>>
>>> All routing tables are created by reading the Routing Policy Database
>>> (RPDB) from the kernel, on systems that support it, and importing only
>>> those tables which are referenced in the RPDB rules with a table lookup
>>> action. The table IDs and rule priority are copied from the kernel RPDB
>>> as is.
>>>
>>> Current implementation only supports RPDB rules with a source address
>>> selector, in form of '[not] from IP' match.
>>>
>>> Signed-off-by: Dima Chumak <[email protected]>
>>> ---
>>>  lib/netdev-dummy.c           |   4 +-
>>>  lib/ovs-router.c             | 208 ++++++++++++++++++++++------
>>>  lib/ovs-router.h             |   7 +
>>>  lib/packets.c                |  20 +++
>>>  lib/packets.h                |   2 +
>>>  lib/route-table.c            | 253 ++++++++++++++++++++++++++++++++---
>>>  lib/route-table.h            |  21 ++-
>>>  tests/ovs-router.at          |  10 --
>>>  tests/test-lib-route-table.c |   5 +-
>>>  9 files changed, 451 insertions(+), 79 deletions(-)
>>>
> 
> <snip>
> 
>>> +    /* Skip standard IPv6 rules for local and main tables to avoid 
>>> duplicates
>>> +     * as they end up being the same as the similar IPv4 rules.
>>> +     */
>>> +    if (!ipv4 && (change->rud.prio == 0 || change->rud.prio == 0x7FFE) &&
>>> +        from_addr_any && is_standard_table_id(change->rud.lookup_table)) {
>>> +        change->relevant = false;
>>>      }
>>
>> Looks like we have a problem here with the rules, but also with multi-table
>> routing as well.
>>
>> For routes we're making sure that IPv4 routes have higher priority with the
>> extra +96 prefix length.  Actual IPv6 addresses do not match them as they
>> are mapped IPv4 and so have special first 96 bits, and IPv4 packets do not
>> match IPv6 routes, unless there are no matches on IPv4, for which there is
>> normally a default route that matches everything, i.e. 0.0.0.0 mapped into
>> IPv6, which is not all-zero.  But that last part breaks as soon as we have
>> multi-table routing.  Because now, if we didn't match anything in table N
>> for IPv4, we will proceed matching IPv6 rules in that table before we get
>> to the table N+1.  And if we have a default IPv6 route in table N, it will
>> swallow IPv4 traffic.
>>
>> Same problem here for rules, 'from all' IPv6 rule will swallow all the IPv4
>> traffic., because it will be a /0 match and we do not distinguish packets
>> based on type.
>>
>> We probably need a flag for a rule that says if it si v4 or v6.  This is
>> also a problem for the user-added rules, as 'from all' doesn't say if its
>> supposed to be v4 or v6, as no address is provided.  The show/add/del
>> commands will need to handle that.
>>
>> For the routes, we may get away with having a single classifier, but we'll
>> need to add a match on ethernet address and put addresses into their
>> corresponding protocol fields, not just use ipv6_src/dst for both.
>>
>> Would be great to have some tests covering such mixed v4+v6 scenarios as
>> we clearly don't have any.
>>
> 
> In the V4 series I used a different approach to matching on IPv[46] routes
> in a single classifier. Please, let me know if that looks good enough or you
> prefer to have it the way you outlined here. In the latter case, could you
> please elaborate a bit more on the suggested approach with using ethernet
> addresses as I'm not very familiar with the classifier design and would
> appreciate some guidance in this area.

Hi.  Sorry for delay.  Last month was a little hectic with the conference
and some time off.

I think, your approach is fine, though there are some corner cases for
rules specifically that may need fixing still.  I'll reply to v4.

FWIW, what I meant was ethertype, not ethernet addresses, sorry.  In other
words, add flow.dl_type match instead of just relying on the ipv6_dst/src
in the flow structure:

 struct flow flow = {
     .dl_type = htons(ETH_TYPE_IP),
     .nw_dst = <ip>,
     .pkt_mark = mark,
 };

for IPv4 and the following for IPv6:

 struct flow flow = {
     .dl_type = htons(ETH_TYPE_IPV6),
     .ipv6_dst = <ip6>,
     .pkt_mark = mark,
 };

Similarly, mask in rt_init_match would consist of dl_type, nw_dst and
pkt_mark for ipv4 rules and dl_type, ipv6_dst and pkt_mark for ipv6.
This way classifier will have an extra field to look at and will be able
to easily distinguish ipv4 and ipv6 entries.  And we would not need to
artificially bump priority of ipv4 rules, they can have priority exactly
equal to their prefix length.  And classifier would never find ipv6 rule
for ipv4 lookup or vice versa.

It will likely be more code to write things this way, and it doesn't seem
necessary, if we can just handle the special case of finding route with
ipv6 address for an ipv4 lookup address as you did in v4 of this patch set.
At least, I can't think of a case where it will not be enough at the moment,
as when it happens, it means that prefix length of this route is smaller
than 96, which can't be a valid prefix for ipv4.

We can switch the implementation later if we find the case that doesn't
work for one reason or another, but it shouldn't be the case.

I'll send some comments for the rule matching in v4.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to