bridge-utils: wrong sysfs path odds

DISCUSSION Richard Weinberger 4 participants 9 messages Nov 25, 2015 1 message not in the archive

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Richard Weinberger

author

2015-11-25 00:15 UTC #0 lore raw

Hi!

Today I was hunting down an issue where "brctl stp br0 off" always failed on mips64be with n32 userland.

It turned out that the ioctl(fd, SIOCDEVPRIVATE, &ifr) with BRCTL_SET_BRIDGE_STP_STATE returned -EOPNOTSUPP.
First I thought that this is a plain ABI issue on mips as in old_dev_ioctl() the ioctl() argument was 0x1 instead of the expected BRCTL_SET_BRIDGE_STP_STATE (0x14)

Further investigation showed that brctl first tries to open the sysfs file "/sys/class/net/br0/stp_state" and falls back to the legacy ioctl() upon failure.

On my mips setup old_dev_ioctl() seems not to work. And the function's comment is correct:
/*
* Legacy ioctl's through SIOCDEVPRIVATE
* This interface is deprecated because it was too difficult to
* to do the translation for 32/64bit ioctl compatibility.
*/

Later I've realized that the sysfs path is wrong, the "bridge/" directory part is missing.
On most setups nobody would notice as the fallback ioctl() works.

Debian's bridge-utils package carries a patch which fixes the sysfs paths. Can we please have this patch also in upstream bridge-utils?

Thanks,
//richard

Richard Weinberger

author

2015-11-25 00:24 UTC #1 lore raw

Am 25.11.2015 um 01:15 schrieb Richard Weinberger:

Quoted 9 lines

Hi!
Today I was hunting down an issue where "brctl stp br0 off" always failed on mips64be with n32 userland.
It turned out that the ioctl(fd, SIOCDEVPRIVATE, &ifr) with BRCTL_SET_BRIDGE_STP_STATE returned -EOPNOTSUPP.
First I thought that this is a plain ABI issue on mips as in old_dev_ioctl() the ioctl() argument was 0x1 instead of the expected BRCTL_SET_BRIDGE_STP_STATE (0x14)

Should be 0xe and not 0x14. It is 14 in decimal. :)

Thanks,
//richard

Stephen Hemminger

2015-11-25 00:37 UTC #2 lore raw

Quoted 11 lines · On Wed, 25 Nov 2015 01:24:47 +0100 Richard Weinberger <[email protected]> wrote:

Am 25.11.2015 um 01:15 schrieb Richard Weinberger:
Quoted 9 lines
Hi!
Today I was hunting down an issue where "brctl stp br0 off" always failed on mips64be with n32 userland.
It turned out that the ioctl(fd, SIOCDEVPRIVATE, &ifr) with BRCTL_SET_BRIDGE_STP_STATE returned -EOPNOTSUPP.
First I thought that this is a plain ABI issue on mips as in old_dev_ioctl() the ioctl() argument was 0x1 instead of the expected BRCTL_SET_BRIDGE_STP_STATE (0x14)

Quoted 4 lines

Should be 0xe and not 0x14. It is 14 in decimal. :)
Thanks,
//richard

Ask Debian maintainer to send his patches, I don't go patch hunting.

Richard Weinberger

author

2015-11-25 00:40 UTC #3 lore raw

Am 25.11.2015 um 01:37 schrieb Stephen Hemminger:

Quoted 14 lines

Quoted 11 lines · On Wed, 25 Nov 2015 01:24:47 +0100 Richard Weinberger <[email protected]> wrote:
Am 25.11.2015 um 01:15 schrieb Richard Weinberger:
Quoted 9 lines
Hi!
Today I was hunting down an issue where "brctl stp br0 off" always failed on mips64be with n32 userland.
It turned out that the ioctl(fd, SIOCDEVPRIVATE, &ifr) with BRCTL_SET_BRIDGE_STP_STATE returned -EOPNOTSUPP.
First I thought that this is a plain ABI issue on mips as in old_dev_ioctl() the ioctl() argument was 0x1 instead of the expected BRCTL_SET_BRIDGE_STP_STATE (0x14)

Quoted 6 lines

Quoted 4 lines
Should be 0xe and not 0x14. It is 14 in decimal. :)
Thanks,
//richard
Ask Debian maintainer to send his patches, I don't go patch hunting.

He is Cc'ed :-)

Thanks,
//richard

Richard Weinberger

author

2015-11-25 09:21 UTC #4 lore raw

In reply to Stephen Hemminger

Am 25.11.2015 um 01:37 schrieb Stephen Hemminger:

Quoted 14 lines

Quoted 11 lines · On Wed, 25 Nov 2015 01:24:47 +0100 Richard Weinberger <[email protected]> wrote:
Am 25.11.2015 um 01:15 schrieb Richard Weinberger:
Quoted 9 lines
Hi!
Today I was hunting down an issue where "brctl stp br0 off" always failed on mips64be with n32 userland.
It turned out that the ioctl(fd, SIOCDEVPRIVATE, &ifr) with BRCTL_SET_BRIDGE_STP_STATE returned -EOPNOTSUPP.
First I thought that this is a plain ABI issue on mips as in old_dev_ioctl() the ioctl() argument was 0x1 instead of the expected BRCTL_SET_BRIDGE_STP_STATE (0x14)

Quoted 7 lines

Quoted 4 lines
Should be 0xe and not 0x14. It is 14 in decimal. :)
Thanks,
//richard
Ask Debian maintainer to send his patches, I don't go patch hunting.

While looking what other distros do I came across this patch:
https://pkgs.fedoraproject.org/cgit/bridge-utils.git/tree/bridge-utils-1.5-check-error-returns-from-write-to-sysfs.patch

Beside of checking return errors is fixes also the sysfs path in br_set(). Can you please merge it upstream?

Distros seems to carry more patches for that package, if it helps I can do the patch hunting for you. It would be nice to have a recent bridge-utils release. The last one is from 2011.

Thanks,
//richard

Richard Weinberger

author

2015-11-25 22:37 UTC #6 lore raw

In reply to a message not in the archive

Hi!

Am 25.11.2015 um 23:30 schrieb Santiago Garcia Mantinan:

Quoted 11 lines

Hi!
Quoted 1 line
Ask Debian maintainer to send his patches, I don't go patch hunting.
While looking what other distros do I came across this patch:
https://pkgs.fedoraproject.org/cgit/bridge-utils.git/tree/bridge-utils-1.5-check-error-returns-from-write-to-sysfs.patch
Don't know about the fedora guys but at least I did send some patches which include this /bridge addition, and Stephen did even ack it as you can read on:
http://lists.linuxfoundation.org/pipermail/bridge/2011-May/007646.html

Meanwhile I've found https://git.kernel.org/cgit/linux/kernel/git/shemminger/bridge-utils.git/
It contains some fixes.
So, the problem is that we have fixes but no release which contains these. Stephen, can you do a maintenance release? Maybe 1.5.1?

Thanks,
//richard

Florian Fainelli

2015-11-25 22:39 UTC #8 lore raw

In reply to Richard Weinberger

Quoted 23 lines · Richard Weinberger

Am 25.11.2015 um 01:37 schrieb Stephen Hemminger:
Quoted 14 lines
Quoted 11 lines · On Wed, 25 Nov 2015 01:24:47 +0100 Richard Weinberger <[email protected]> wrote:
Am 25.11.2015 um 01:15 schrieb Richard Weinberger:
Quoted 9 lines
Hi!
Today I was hunting down an issue where "brctl stp br0 off" always failed on mips64be with n32 userland.
It turned out that the ioctl(fd, SIOCDEVPRIVATE, &ifr) with BRCTL_SET_BRIDGE_STP_STATE returned -EOPNOTSUPP.
First I thought that this is a plain ABI issue on mips as in old_dev_ioctl() the ioctl() argument was 0x1 instead of the expected BRCTL_SET_BRIDGE_STP_STATE (0x14)
Quoted 7 lines
Quoted 4 lines
Should be 0xe and not 0x14. It is 14 in decimal. :)
Thanks,
//richard
Ask Debian maintainer to send his patches, I don't go patch hunting.

Quoted 8 lines

While looking what other distros do I came across this patch:
https://pkgs.fedoraproject.org/cgit/bridge-utils.git/tree/bridge-utils-1.5-check-error-returns-from-write-to-sysfs.patch
Beside of checking return errors is fixes also the sysfs path in br_set(). Can you please merge it upstream?
Distros seems to carry more patches for that package, if it helps I can do the patch hunting for you. It would be nice to have a recent bridge-utils release. The last one is from 2011.

Most of what bridge-utils does can be done by iproute2's bridge sub-command FWIW.

Signature · 1 line

Florian

Richard Weinberger

author

2015-11-25 22:41 UTC #9 lore raw

Am 25.11.2015 um 23:38 schrieb Florian Fainelli:

Quoted 27 lines · Richard Weinberger

Quoted 26 lines
Am 25.11.2015 um 01:37 schrieb Stephen Hemminger:
Quoted 14 lines
Quoted 11 lines · On Wed, 25 Nov 2015 01:24:47 +0100 Richard Weinberger <[email protected]> wrote:
Am 25.11.2015 um 01:15 schrieb Richard Weinberger:
Quoted 9 lines
Hi!
Today I was hunting down an issue where "brctl stp br0 off" always failed on mips64be with n32 userland.
It turned out that the ioctl(fd, SIOCDEVPRIVATE, &ifr) with BRCTL_SET_BRIDGE_STP_STATE returned -EOPNOTSUPP.
First I thought that this is a plain ABI issue on mips as in old_dev_ioctl() the ioctl() argument was 0x1 instead of the expected BRCTL_SET_BRIDGE_STP_STATE (0x14)
Quoted 7 lines
Quoted 4 lines
Should be 0xe and not 0x14. It is 14 in decimal. :)
Thanks,
//richard
Ask Debian maintainer to send his patches, I don't go patch hunting.
While looking what other distros do I came across this patch:
https://pkgs.fedoraproject.org/cgit/bridge-utils.git/tree/bridge-utils-1.5-check-error-returns-from-write-to-sysfs.patch

Quoted 8 lines

Quoted 5 lines
Beside of checking return errors is fixes also the sysfs path in br_set(). Can you please merge it upstream?
Distros seems to carry more patches for that package, if it helps I can do the patch hunting for you. It would be nice to have a recent bridge-utils release. The last one is from 2011.
Most of what bridge-utils does can be done by iproute2's bridge sub-command FWIW.

Sure, but a lot of userspace still depends on brctl.
And IMHO brctl is magnitudes easier to use than iproute's bridge tool.

Thanks,
//richard

Santiago Garcia Mantinan

2015-11-25 22:44 UTC #7 lore raw

In reply to a message not in the archive

(Message resent due to list rejection)

Hi!

Quoted 6 lines

Ask Debian maintainer to send his patches, I don't go patch hunting.
While looking what other distros do I came across this patch:
https://pkgs.fedoraproject.org/cgit/bridge-utils.git/tree/bridge-utils-1.5-check-error-returns-from-write-to-sysfs.patch

Don't know about the fedora guys but at least I did send some patches which include this /bridge addition, and Stephen did even ack it as you can read on:
http://lists.linuxfoundation.org/pipermail/bridge/2011-May/007646.html

Regards.

Signature · 1 line

Manty/BestiaTester -> http://manty.net