Message ID | 20240827111813.2115285-1-idosch@nvidia.com (mailing list archive) |
---|---|
Headers | show |
Series | Unmask upper DSCP bits - part 2 | expand |
On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > tl;dr - This patchset continues to unmask the upper DSCP bits in the > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > DSCP. No functional changes are expected. Part 1 was merged in commit > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > lookup to match against the TOS selector in FIB rules and routes. > > It is currently impossible for user space to configure FIB rules that > match on the DSCP value as the upper DSCP bits are either masked in the > various call sites that initialize the IPv4 flow key or along the path > to the FIB core. > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be necessary as IPv6 already takes all the DSCP bits into account. Also we don't need to keep any compatibility with the legacy TOS interpretation, as it has never been defined nor used in IPv6. > need to make sure the entire DSCP value is present in the IPv4 flow key. > This patchset continues to unmask the upper DSCP bits, but this time in > the output route path.
On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > DSCP. No functional changes are expected. Part 1 was merged in commit > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > lookup to match against the TOS selector in FIB rules and routes. > > > > It is currently impossible for user space to configure FIB rules that > > match on the DSCP value as the upper DSCP bits are either masked in the > > various call sites that initialize the IPv4 flow key or along the path > > to the FIB core. > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > necessary as IPv6 already takes all the DSCP bits into account. Also we > don't need to keep any compatibility with the legacy TOS interpretation, > as it has never been defined nor used in IPv6. Yes. I want to add the DSCP selector for both families so that user space would not need to use different selectors for different families. It's implemented in the patches I previously shared: https://github.com/idosch/linux/commit/a3289a6838a0d0e6e0a30a61132bdce3d2f71a3c.patch https://github.com/idosch/linux/commit/ff5dd634fb278431b58437654d7f65b57fd4ae4b.patch https://github.com/idosch/linux/commit/3060ecb534475eadabfa1d419dd64804f0bd0148.patch https://github.com/idosch/linux/commit/12ddbce4f519b42477ea1e130b6d2bab1cca137c.patch > > > need to make sure the entire DSCP value is present in the IPv4 flow key. > > This patchset continues to unmask the upper DSCP bits, but this time in > > the output route path. >
On Tue, Aug 27, 2024 at 06:45:53PM +0300, Ido Schimmel wrote: > On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > > DSCP. No functional changes are expected. Part 1 was merged in commit > > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > > lookup to match against the TOS selector in FIB rules and routes. > > > > > > It is currently impossible for user space to configure FIB rules that > > > match on the DSCP value as the upper DSCP bits are either masked in the > > > various call sites that initialize the IPv4 flow key or along the path > > > to the FIB core. > > > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > > necessary as IPv6 already takes all the DSCP bits into account. Also we > > don't need to keep any compatibility with the legacy TOS interpretation, > > as it has never been defined nor used in IPv6. > > Yes. I want to add the DSCP selector for both families so that user > space would not need to use different selectors for different families. Another approach could be to add a mask to the existing tos/dsfield. For example: # ip -4 rule add dsfield 0x04/0xfc table 100 # ip -6 rule add dsfield 0xf8/0xfc table 100 The default IPv4 mask (when user doesn't specify one) would be 0x1c and the default IPv6 mask would be 0xfc. WDYT? > It's implemented in the patches I previously shared: > > https://github.com/idosch/linux/commit/a3289a6838a0d0e6e0a30a61132bdce3d2f71a3c.patch > https://github.com/idosch/linux/commit/ff5dd634fb278431b58437654d7f65b57fd4ae4b.patch > https://github.com/idosch/linux/commit/3060ecb534475eadabfa1d419dd64804f0bd0148.patch > https://github.com/idosch/linux/commit/12ddbce4f519b42477ea1e130b6d2bab1cca137c.patch > > > > > > need to make sure the entire DSCP value is present in the IPv4 flow key. > > > This patchset continues to unmask the upper DSCP bits, but this time in > > > the output route path. > >
On Tue, Aug 27, 2024 at 06:45:53PM +0300, Ido Schimmel wrote: > On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > > DSCP. No functional changes are expected. Part 1 was merged in commit > > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > > lookup to match against the TOS selector in FIB rules and routes. > > > > > > It is currently impossible for user space to configure FIB rules that > > > match on the DSCP value as the upper DSCP bits are either masked in the > > > various call sites that initialize the IPv4 flow key or along the path > > > to the FIB core. > > > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > > necessary as IPv6 already takes all the DSCP bits into account. Also we > > don't need to keep any compatibility with the legacy TOS interpretation, > > as it has never been defined nor used in IPv6. > > Yes. I want to add the DSCP selector for both families so that user > space would not need to use different selectors for different families. > It's implemented in the patches I previously shared: Hum, I guess that was a misunderstanding on my side. I read "adding a DSCP selector to [IPv4 and] IPv6 FIB rules" as "adding the possibility to match only the 3-bits TOS in fib6_rules". But your fib6_rule.c patch doesn't modify fib6_rule_match(), so I believe that what you really meant was just to add the new FRA_DSCP netlink attribute to IPv6. Am I getting it right? > https://github.com/idosch/linux/commit/a3289a6838a0d0e6e0a30a61132bdce3d2f71a3c.patch > https://github.com/idosch/linux/commit/ff5dd634fb278431b58437654d7f65b57fd4ae4b.patch > https://github.com/idosch/linux/commit/3060ecb534475eadabfa1d419dd64804f0bd0148.patch > https://github.com/idosch/linux/commit/12ddbce4f519b42477ea1e130b6d2bab1cca137c.patch > > > need to make sure the entire DSCP value is present in the IPv4 flow key. > > > This patchset continues to unmask the upper DSCP bits, but this time in > > > the output route path. > > >
On Wed, Aug 28, 2024 at 03:09:19PM +0300, Ido Schimmel wrote: > On Tue, Aug 27, 2024 at 06:45:53PM +0300, Ido Schimmel wrote: > > On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > > > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > > > DSCP. No functional changes are expected. Part 1 was merged in commit > > > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > > > lookup to match against the TOS selector in FIB rules and routes. > > > > > > > > It is currently impossible for user space to configure FIB rules that > > > > match on the DSCP value as the upper DSCP bits are either masked in the > > > > various call sites that initialize the IPv4 flow key or along the path > > > > to the FIB core. > > > > > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > > > > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > > > necessary as IPv6 already takes all the DSCP bits into account. Also we > > > don't need to keep any compatibility with the legacy TOS interpretation, > > > as it has never been defined nor used in IPv6. > > > > Yes. I want to add the DSCP selector for both families so that user > > space would not need to use different selectors for different families. > > Another approach could be to add a mask to the existing tos/dsfield. For > example: > > # ip -4 rule add dsfield 0x04/0xfc table 100 > # ip -6 rule add dsfield 0xf8/0xfc table 100 > > The default IPv4 mask (when user doesn't specify one) would be 0x1c and > the default IPv6 mask would be 0xfc. > > WDYT? For internal implementation, I find the mask option elegant (to avoid conditionals). But I don't really like the idea of letting user space provide its own mask. This would let the user create non-standard behaviours, likely by mistake (as nobody seem to ever have requested that flexibility). I think my favourite approach would be to have the new FRA_DSCP attribute work identically on both IPv4 and IPv6 FIB rules and keep the behaviour of the old "tos" field of struct fib_rule_hdr unchanged. This "tos" field would still work differently for IPv4 and IPv6, as it always did, but people wanting consistent behaviour could just use FRA_DSCP instead. Also, FRA_DSCP accepts real DSCP values as defined in RFCs, while "tos" requires the 2 bits shift. For all these reasons, I'm tempted to just consider "tos" as a legacy option used only for backward compatibility, while FRA_DSCP would be the "clean" interface. Is that approach acceptable for you?
On Thu, Aug 29, 2024 at 01:30:58PM +0200, Guillaume Nault wrote: > On Tue, Aug 27, 2024 at 06:45:53PM +0300, Ido Schimmel wrote: > > On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > > > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > > > DSCP. No functional changes are expected. Part 1 was merged in commit > > > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > > > lookup to match against the TOS selector in FIB rules and routes. > > > > > > > > It is currently impossible for user space to configure FIB rules that > > > > match on the DSCP value as the upper DSCP bits are either masked in the > > > > various call sites that initialize the IPv4 flow key or along the path > > > > to the FIB core. > > > > > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > > > > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > > > necessary as IPv6 already takes all the DSCP bits into account. Also we > > > don't need to keep any compatibility with the legacy TOS interpretation, > > > as it has never been defined nor used in IPv6. > > > > Yes. I want to add the DSCP selector for both families so that user > > space would not need to use different selectors for different families. > > It's implemented in the patches I previously shared: > > Hum, I guess that was a misunderstanding on my side. I read > "adding a DSCP selector to [IPv4 and] IPv6 FIB rules" as "adding the > possibility to match only the 3-bits TOS in fib6_rules". But your > fib6_rule.c patch doesn't modify fib6_rule_match(), so I believe that > what you really meant was just to add the new FRA_DSCP netlink > attribute to IPv6. Am I getting it right? Yes. To be clear, you will be able to use the new 'dscp' keyword exactly the same way with both IPv4 and IPv6: # ip -4 rule add dscp 63 table 100 # ip -6 rule add dscp 63 table 100 Mixing 'dscp' and 'tos' will not work: # ip -4 rule add dscp 7 tos 0x1c table 100 Error: Cannot specify both TOS and DSCP. # ip -6 rule add dscp 7 tos 0x1c table 100 Error: Cannot specify both TOS and DSCP. > > > https://github.com/idosch/linux/commit/a3289a6838a0d0e6e0a30a61132bdce3d2f71a3c.patch > > https://github.com/idosch/linux/commit/ff5dd634fb278431b58437654d7f65b57fd4ae4b.patch > > https://github.com/idosch/linux/commit/3060ecb534475eadabfa1d419dd64804f0bd0148.patch > > https://github.com/idosch/linux/commit/12ddbce4f519b42477ea1e130b6d2bab1cca137c.patch > > > > > > need to make sure the entire DSCP value is present in the IPv4 flow key. > > > > This patchset continues to unmask the upper DSCP bits, but this time in > > > > the output route path. > > > > > >
On Thu, Aug 29, 2024 at 01:54:46PM +0200, Guillaume Nault wrote: > On Wed, Aug 28, 2024 at 03:09:19PM +0300, Ido Schimmel wrote: > > On Tue, Aug 27, 2024 at 06:45:53PM +0300, Ido Schimmel wrote: > > > On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > > > > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > > > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > > > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > > > > DSCP. No functional changes are expected. Part 1 was merged in commit > > > > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > > > > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > > > > lookup to match against the TOS selector in FIB rules and routes. > > > > > > > > > > It is currently impossible for user space to configure FIB rules that > > > > > match on the DSCP value as the upper DSCP bits are either masked in the > > > > > various call sites that initialize the IPv4 flow key or along the path > > > > > to the FIB core. > > > > > > > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > > > > > > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > > > > necessary as IPv6 already takes all the DSCP bits into account. Also we > > > > don't need to keep any compatibility with the legacy TOS interpretation, > > > > as it has never been defined nor used in IPv6. > > > > > > Yes. I want to add the DSCP selector for both families so that user > > > space would not need to use different selectors for different families. > > > > Another approach could be to add a mask to the existing tos/dsfield. For > > example: > > > > # ip -4 rule add dsfield 0x04/0xfc table 100 > > # ip -6 rule add dsfield 0xf8/0xfc table 100 > > > > The default IPv4 mask (when user doesn't specify one) would be 0x1c and > > the default IPv6 mask would be 0xfc. > > > > WDYT? > > For internal implementation, I find the mask option elegant (to avoid > conditionals). But I don't really like the idea of letting user space > provide its own mask. This would let the user create non-standard > behaviours, likely by mistake (as nobody seem to ever have requested > that flexibility). > > I think my favourite approach would be to have the new FRA_DSCP > attribute work identically on both IPv4 and IPv6 FIB rules and keep > the behaviour of the old "tos" field of struct fib_rule_hdr unchanged. > > This "tos" field would still work differently for IPv4 and IPv6, as it > always did, but people wanting consistent behaviour could just use > FRA_DSCP instead. Also, FRA_DSCP accepts real DSCP values as defined in > RFCs, while "tos" requires the 2 bits shift. For all these reasons, I'm > tempted to just consider "tos" as a legacy option used only for > backward compatibility, while FRA_DSCP would be the "clean" interface. > > Is that approach acceptable for you? Yes. The patches I shared implement this approach :)
On Thu, Aug 29, 2024 at 05:43:17PM +0300, Ido Schimmel wrote: > On Thu, Aug 29, 2024 at 01:30:58PM +0200, Guillaume Nault wrote: > > On Tue, Aug 27, 2024 at 06:45:53PM +0300, Ido Schimmel wrote: > > > On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > > > > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > > > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > > > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > > > > DSCP. No functional changes are expected. Part 1 was merged in commit > > > > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > > > > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > > > > lookup to match against the TOS selector in FIB rules and routes. > > > > > > > > > > It is currently impossible for user space to configure FIB rules that > > > > > match on the DSCP value as the upper DSCP bits are either masked in the > > > > > various call sites that initialize the IPv4 flow key or along the path > > > > > to the FIB core. > > > > > > > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > > > > > > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > > > > necessary as IPv6 already takes all the DSCP bits into account. Also we > > > > don't need to keep any compatibility with the legacy TOS interpretation, > > > > as it has never been defined nor used in IPv6. > > > > > > Yes. I want to add the DSCP selector for both families so that user > > > space would not need to use different selectors for different families. > > > It's implemented in the patches I previously shared: > > > > Hum, I guess that was a misunderstanding on my side. I read > > "adding a DSCP selector to [IPv4 and] IPv6 FIB rules" as "adding the > > possibility to match only the 3-bits TOS in fib6_rules". But your > > fib6_rule.c patch doesn't modify fib6_rule_match(), so I believe that > > what you really meant was just to add the new FRA_DSCP netlink > > attribute to IPv6. Am I getting it right? > > Yes. To be clear, you will be able to use the new 'dscp' keyword exactly > the same way with both IPv4 and IPv6: > > # ip -4 rule add dscp 63 table 100 > # ip -6 rule add dscp 63 table 100 > > Mixing 'dscp' and 'tos' will not work: > > # ip -4 rule add dscp 7 tos 0x1c table 100 > Error: Cannot specify both TOS and DSCP. > # ip -6 rule add dscp 7 tos 0x1c table 100 > Error: Cannot specify both TOS and DSCP. Thanks, that's exactly what I had in mind. > > > > > https://github.com/idosch/linux/commit/a3289a6838a0d0e6e0a30a61132bdce3d2f71a3c.patch > > > https://github.com/idosch/linux/commit/ff5dd634fb278431b58437654d7f65b57fd4ae4b.patch > > > https://github.com/idosch/linux/commit/3060ecb534475eadabfa1d419dd64804f0bd0148.patch > > > https://github.com/idosch/linux/commit/12ddbce4f519b42477ea1e130b6d2bab1cca137c.patch > > > > > > > > > need to make sure the entire DSCP value is present in the IPv4 flow key. > > > > > This patchset continues to unmask the upper DSCP bits, but this time in > > > > > the output route path. > > > > > > > > > >
On Thu, Aug 29, 2024 at 05:52:05PM +0300, Ido Schimmel wrote: > On Thu, Aug 29, 2024 at 01:54:46PM +0200, Guillaume Nault wrote: > > On Wed, Aug 28, 2024 at 03:09:19PM +0300, Ido Schimmel wrote: > > > On Tue, Aug 27, 2024 at 06:45:53PM +0300, Ido Schimmel wrote: > > > > On Tue, Aug 27, 2024 at 03:47:05PM +0200, Guillaume Nault wrote: > > > > > On Tue, Aug 27, 2024 at 02:18:01PM +0300, Ido Schimmel wrote: > > > > > > tl;dr - This patchset continues to unmask the upper DSCP bits in the > > > > > > IPv4 flow key in preparation for allowing IPv4 FIB rules to match on > > > > > > DSCP. No functional changes are expected. Part 1 was merged in commit > > > > > > ("Merge branch 'unmask-upper-dscp-bits-part-1'"). > > > > > > > > > > > > The TOS field in the IPv4 flow key ('flowi4_tos') is used during FIB > > > > > > lookup to match against the TOS selector in FIB rules and routes. > > > > > > > > > > > > It is currently impossible for user space to configure FIB rules that > > > > > > match on the DSCP value as the upper DSCP bits are either masked in the > > > > > > various call sites that initialize the IPv4 flow key or along the path > > > > > > to the FIB core. > > > > > > > > > > > > In preparation for adding a DSCP selector to IPv4 and IPv6 FIB rules, we > > > > > > > > > > Hum, do you plan to add a DSCP selector for IPv6? That shouldn't be > > > > > necessary as IPv6 already takes all the DSCP bits into account. Also we > > > > > don't need to keep any compatibility with the legacy TOS interpretation, > > > > > as it has never been defined nor used in IPv6. > > > > > > > > Yes. I want to add the DSCP selector for both families so that user > > > > space would not need to use different selectors for different families. > > > > > > Another approach could be to add a mask to the existing tos/dsfield. For > > > example: > > > > > > # ip -4 rule add dsfield 0x04/0xfc table 100 > > > # ip -6 rule add dsfield 0xf8/0xfc table 100 > > > > > > The default IPv4 mask (when user doesn't specify one) would be 0x1c and > > > the default IPv6 mask would be 0xfc. > > > > > > WDYT? > > > > For internal implementation, I find the mask option elegant (to avoid > > conditionals). But I don't really like the idea of letting user space > > provide its own mask. This would let the user create non-standard > > behaviours, likely by mistake (as nobody seem to ever have requested > > that flexibility). > > > > I think my favourite approach would be to have the new FRA_DSCP > > attribute work identically on both IPv4 and IPv6 FIB rules and keep > > the behaviour of the old "tos" field of struct fib_rule_hdr unchanged. > > > > This "tos" field would still work differently for IPv4 and IPv6, as it > > always did, but people wanting consistent behaviour could just use > > FRA_DSCP instead. Also, FRA_DSCP accepts real DSCP values as defined in > > RFCs, while "tos" requires the 2 bits shift. For all these reasons, I'm > > tempted to just consider "tos" as a legacy option used only for > > backward compatibility, while FRA_DSCP would be the "clean" interface. > > > > Is that approach acceptable for you? > > Yes. The patches I shared implement this approach :) Thanks for confirming. And sorry for the misunderstanding in v1.