Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iproute-extension breaks fwmark #9

Open
SriramScorp opened this issue Nov 23, 2021 · 2 comments
Open

iproute-extension breaks fwmark #9

SriramScorp opened this issue Nov 23, 2021 · 2 comments

Comments

@SriramScorp
Copy link

When adding fwmark rules using this extension, rules seem to have the same value upon listing. However, when rules are listed with the built-in iproute2 binary, they display the correct values. Current working option is to use this extension only for turning MPTCP on or off while falling back to the original, unmodified iproute2 for rule creation and selection.

root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# dpkg -l | grep iproute2
ii  iproute2                  4.20.0-2+deb10u1            armhf        networking and traffic control tools
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# which ip
/sbin/ip
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# ip -V
ip utility, iproute2-ss190107
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# ./ip/ip -V
ip utility, iproute2-ss181023
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# ./ip/ip rule add fwmark 5 table 5
root@raspi-mptcp:/tmp/iproute-mptcp# ./ip/ip rule add fwmark 6 table 6
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# ./ip/ip rule list 
0:	from all lookup local
32764:	from all fwmark 0x9076c lookup 6
32765:	from all fwmark 0x9076c lookup 5
32766:	from all lookup main
32767:	from all lookup default
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# ip rule list
0:	from all lookup local
32764:	from all fwmark 0x6 lookup 6
32765:	from all fwmark 0x5 lookup 5
32766:	from all lookup main
32767:	from all lookup default
root@raspi-mptcp:/tmp/iproute-mptcp# 
@matttbe
Copy link
Member

matttbe commented Nov 23, 2021

Hi @SriramScorp,

Could you eventually compile a more recent version?

I don't see why this issue would come from the modifications we did to support the new multipath parameter for ip link.

Feel free to try master branch (or mptcp_v0.96 or mptcp_v0.95) but the upstream project recommendations is to use the latest version as older ones are no longer maintained and the latest one should still be compatible with old kernels.

@SriramScorp
Copy link
Author

The kernel being used is the mptcp kernel built on a fairly recent RaspiOS (2021-01-11-raspios-buster-armhf.img).

root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# uname -r
5.4.83-MPTCP+
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# lsb_release -a
No LSB modules are available.
Distributor ID:	Raspbian
Description:	Raspbian GNU/Linux 10 (buster)
Release:	10
Codename:	buster
root@raspi-mptcp:/tmp/iproute-mptcp# 

Faced the issue when I built the extension from the latest commit of the default branch (mptcp_v0.95). After recompiling with the master branch, it seems to works just fine.

root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# ./ip/ip -V
ip utility, iproute2-5.15.0
root@raspi-mptcp:/tmp/iproute-mptcp# 
root@raspi-mptcp:/tmp/iproute-mptcp# ./ip/ip rule list
0:	from all lookup local
32764:	from all fwmark 0x6 lookup 6
32765:	from all fwmark 0x5 lookup 5
32766:	from all lookup main
32767:	from all lookup default
root@raspi-mptcp:/tmp/iproute-mptcp# 

I guess README should mention a recommendation to use master branch since it's currently not the default one.

arter97 pushed a commit to arter97/iproute-mptcp that referenced this issue Jan 27, 2023
Petr Machata  says:

====================

A new rtnetlink message, RTM_SETSTATS, has been added recently in kernel
commit ca0a53dcec94 ("Merge branch 'net-hw-counters-for-soft-devices'").

At the same time, RTM_GETSTATS has been around for a while. The users of
this API are spread in a couple different places: "ip link xstats" reads
stats from the IFLA_STATS_LINK_XSTATS and _XSTATS_SLAVE subgroups, "ip
link afstats" then reads IFLA_STATS_AF_SPEC.

Finally, to read IFLA_STATS_LINK_OFFLOAD_XSTATS, one would use ifstats.
This does not seem to be a good fit for IFLA_OFFLOAD_XSTATS_HW_S_INFO in
particular.

The obvious place to expose all these offload stats suites would be
under a new link subcommand "ip link offload_xstats", or similar, which
would then have syntax for both showing stats and setting them.

However, this looks like a good opportunity to introduce a new top-level
command, "ip stats", that would be the go-to place to access anything
backed by RTM_GETSTATS and RTM_SETSTATS.

This patchset therefore does the following:

- It adds the new "stats" infrastructure

- It adds specifically the ability to toggle and show the suites that
  were recently added to Linux, IFLA_OFFLOAD_XSTATS_HW_S_INFO and
  IFLA_OFFLOAD_XSTATS_L3_STATS.

- It adds support to dump IFLA_OFFLOAD_XSTATS_CPU_HIT, which was not
  available under "ip" at all.

- Does all this in a way that is easy to extend for new stats suites.

The patchset proceeds as follows:

- Patches multipath-tcp#1 and multipath-tcp#2 lay some groundwork and tweak existing code.

- Patch multipath-tcp#3 adds the shell of the new "ip stats" command.

- Patch multipath-tcp#4 adds "ip stats set" and the ability to toggle l3_stats in
  particular.

- Patch multipath-tcp#5 adds "ip stats show", but no actual stats suites.

- Patches multipath-tcp#6-multipath-tcp#9 add support for showing individual stats suites:
  respectively, IFLA_STATS_LINK_64, IFLA_OFFLOAD_XSTATS_CPU_HIT,
  IFLA_OFFLOAD_XSTATS_HW_S_INFO and IFLA_OFFLOAD_XSTATS_L3_STATS.

- Patch multipath-tcp#10 adds support for monitoring stats events to "ip monitor".

- Patch #11 adds man page verbiage for the above.

The plan is to contribute support for afstats and xstats in a follow-up
patch set.

====================

Signed-off-by: David Ahern <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants