sauce has quit [Remote host closed the connection]
<jow>
nbd: ping
<jow>
nbd: we do have a long running bug in the uci ppp* configuration, a conflict in handling "option mtu" in "config interface" sections
<jow>
nbd: netifd applies "option mtu" to the device referenced by "config interface"
<jow>
nbd: pppd also uses the mtu value as mtu and mru cli argument
<jow>
nbd: in practice this means that netifd lowers the ethernet device mtu to the uci value, then launches pppd with cli options mru and mtu set to the same uci value
<jow>
nbd: since the underlying ethernet interface has had its mtu lowered by netifd already before pppd was lanuched, pppd will take the supplied mtu/mru value and substract 8 byte overhead
<jow>
nbd: one simple fix would be renaming the ppp level mtu uci option to something like "ppp_mtu"
<jow>
but to avoid option proliferation and to do what one intuitively expects, I'd rather fix it in a way where netifd ignores "option mtu" for proto ppp(oe) interfaces
goliath has quit [Quit: SIGSEGV]
<jow>
nbd: do you see any clean approach for that, short of hacking a "strcmp(proto, "pppoe") != 0" into netifd?
<jow>
one could also argue that device level options such as mtu should only be handled for the interface protocols none, static, dhcp and dhcpv6 since all other protocols are likely forms of tunneling protocols
<dwfreed>
there are pppoe implementations that allow what's called "baby jumbo frames" where the ethernet mtu is 1508, so that the packet inside pppoe can still be 1500
<jow>
dwfreed: yes, that should work however
<jow>
the situation is just that right now an "option mtu" always result in that mtu on the ethernet dev and that mtu minus 8 on the pppoe dev
<jow>
what is not possible is specifying a pppoe mtu lower than the ethernet mtu
<jow>
setting "option mtu 1508" should to the right thing
<jow>
it will set ethernet to 1508 and pass mru/mtu 1508 to pppd which will cap it to 1500
<jow>
effectively rendering the mtu/mru cli options ineffective
<jow>
but the end results works by accident
<f00b4r0>
dwfreed, jow: baby jumbo frames do work on PPPoE, I use them
<jow>
f00b4r0: yes, I did not say that they do not work
<jow>
rn, pppoe will always used configured mtu minus 8
<f00b4r0>
*nod*, just wanted to confirm :)
<jow>
despite ppp.sh implementing logic to allow for deviating mtu values
<jow>
which is defunct because netifd shadows it
<jow>
we could also live with it but that would make pppoe behavior deviate from pppoa or ppp
<jow>
(deviate as in pppoe has a -8 offset while other ppp family protocols do not)
<jow>
on top of that situation comes that per-interface ethernet-level option mtu is deprecated anyway in favor to option mtu in config device
<jow>
so we have a deprecated layer 2 option declared in a layer 3 interface config block shadowing a protocol level option of the same name
<nbd>
jow: how about allowing pppoe.sh protocol registration to define a 'mtu_offset' property?
<nbd>
it would be a bit complex on the backend, but would make the config simple
<jow>
nbd: that would not solve the case that the configured mtu is applied simultaneously to both the layer2 and the layer3 device
<jow>
e.g. it's impossible to keep the eth at 1500 and lower the ppp one to 1300
<jow>
they'd always change in lock step
<jow>
I thought about a new proto flag maybe, something like spawns_own_device
goliath has joined #openwrt-devel
<nbd>
the same applies to multiple regular interfaces (static, dhcp) referencing the same device
<jow>
true. maybe we should just deprepcated interface level netdev options like mtu?
<jow>
*deprecate
landswipe has joined #openwrt-devel
<landswipe>
Hi everyone, there is an older kmod-spi-gpio-custom referenced in a number of places, and it seems to be deprecated in modern builds.
<nbd>
i guess the other option would be to turn mtu into a l3 property, put it in struct device_user and recalc mtu on updates
<jow>
what makes the pppoe case special is that there's no way to untangle it
<landswipe>
what is the modern way of doing this?
<jow>
for multiple porotos using the same device I could opt for moving my mtu settings to config device and omit it from the protos
<nbd>
turning mtu in to a l3 property has the advantage of dealing with pppoe mtu > 1492 when the device comes up with 1500 by default
<nbd>
without needing to add an extra device-level option
<nbd>
though i guess the extra complexity might not be worth it
<jow>
hm, yeah - higher mtu's are also an aspect I didn't fully thought trough
<jow>
now you can simply put a high (>1500) mtu (and take the -8 offset into consideration) and both the ethernet and the ppp device mtu get bumped
<jow>
if done properly, one would need to change it in two places (once in config device for the ethernet, then in config interface for pppoe)
<jow>
technically it would be correct, but from a ux pov it would be annyoing
<jow>
so maybe we should bite the bullet, rename ppp.sh's mtu to ppp_mtu amd be done with it
<jow>
it just triggers my ocd because a few selected protocols would then use a differently named mtu option to set the mtu
rmilecki has quit [Quit: Konversation terminated!]
rmilecki has joined #openwrt-devel
<nbd>
jow: or how about this - do not parse mtu as device level property if the protocol handler defines it as a property
<nbd>
that way no ui changes are required
<jow>
nbd: that sounds like sane fix. So to retain the current behaviour of bumping (but not lowering) the underlying eth mtu as well, ppp.sh would need to deal with the ethernet mtu?
<jow>
would it do it directly (ip link ...) or via a callback to netifd?
<jow>
actually forget about this, no autp-bumping. If a user wants jumbo stuff, he simply should up the mtu in the device settings and pppd will follow suit
<jow>
we should remove the ${mtu:-1492} from ppp.sh though
<jow>
and simply not pass anything to pppd, it is intelligent enough to figure out sane defaults by itself
<jow>
great. could you give a hint on how to approach the netifd side? Will try to establish a pppoe testbed tonight to try this all out (my vdsl line does not do jumbo frames so I need to test this elsewhere)
zer0def has quit [Quit: zer0def]
<nbd>
took a look at the code, i think it's probably better to make an explicit proto handler flag which tells netifd not to apply device config from the interface settings
<nbd>
otherwise it gets too complex or hackish for such a simple thing