Thread (12 messages) flat view 12 messages, 5 authors, 2015-10-30

Re: [PATCH net-next] ipv6: recreate ipv6 link-local addresses when increasing MTU over IPV6_MIN_MTU

From: Hannes Frederic Sowa <hidden>
Date: 2015-10-26 17:21:11

Hi Alex,

On Mon, Oct 26, 2015, at 18:07, Alexander Duyck wrote:
quoted
quoted
Seems like this code isn't quite correct.  You are calling ipv6_add_dev
for slave devices, and if I understand things correctly I don't believe
that was happening before and may be an unintended side effect.
Hmm, could you quickly help me where I get into this situation? I made
sure I enter the NETDEV_UP part before the IFF_SLAVE test and
disable_ipv6 te
I think I was getting a bit a head of myself.  I was looking over the 
NETDEV_UP code and thinking that we could just fall into that path since 
it is already calling ipv6_add_dev.  However now I am wondering if maybe 
we need to look at adding an idev allocation somewhere before the 
disable_ipv6 check.  I assume that is why you were allocating the idev 
before you were getting into NETDEV_UP?
The original bug report was:

If user reduces the MTU below IPV6_MIN_MTU we addrconf_ifdown the
interface but don't reinitialize the interface if the MTU is increased
later on.
quoted
quoted
You might want to instead just make it so that you only do the jump, and
perhaps change the code in the NETDEV_UP/NETDEV_CHANGE section so that
you test for NETDEV_CHANGE instead of NETDEV_UP.  That should be enough
to get the effect you are looking for and I believe there would be no
change to behaviour other than adding IPv6 link-local addresses when the
MTU is increased.

Give me a bit and I can submit an alternative that may actually work out
a bit better I think.
If you go the NETDEV_CHANGE route instead of NETDEV_UP, you end up with
the IF_READY flag already set from ipv6_add_dev and thus won't do any
initialization of the device.
What I meant was that you don't need to change the event.  If you change 
the check inside the NETDEV_UP/CHANGE code path so that it tests for 
event != NETDEV_CHANGE instead of event == NETDEV_UP you don't need to 
change the event type.
Yeah, that would be possible, too. I just find an equal easier to
follow. ;)
quoted
Sure, I wait.
Might be a bit longer.  I just realized that I think there is another 
bug here where you are going through the NETDEV_UP path even though the 
interface isn't up.  I'll run through some testing this morning to work 
out the kinks.
Ok, cool. I have a look at it again, too.

Bye,
Hannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help