netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dcbw@redhat.com>
To: Tom Gundersen <teg@jklm.no>
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	"olaf@aepfle.de" <olaf@aepfle.de>,
	Richard Weinberger <richard.weinberger@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"driverdev-devel@linuxdriverproject.org"
	<driverdev-devel@linuxdriverproject.org>,
	LKML <linux-kernel@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	"Yue Zhang \(OSTC DEV\)" <yuezha@microsoft.com>,
	Thomas Shao <huishao@microsoft.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Haiyang Zhang <haiyangz@microsoft.com>
Subject: Re: [PATCH] Hyperv: Trigger DHCP renew after host hibernation
Date: Wed, 13 Aug 2014 08:34:48 -0500	[thread overview]
Message-ID: <1407936888.1825.0.camel@dcbw.local> (raw)
In-Reply-To: <CAG-2HqWZtOk=mnsaAh5-zpWG-fv=Omq43GV9dMKHNfr1xEFQTw@mail.gmail.com>

On Wed, 2014-08-13 at 15:15 +0200, Tom Gundersen wrote:
> On Tue, Aug 12, 2014 at 10:29 AM, Dexuan Cui <decui@microsoft.com> wrote:
> >> From: Tom Gundersen
> >> > Unluckily this logic doesn't work because the user-space daemons
> >> > like ifplugd, usually don't renew the DHCP immediately as long as they
> >> > receive a link-down message: they usually wait for some seconds and if
> >> > they find the link becomes up soon, they won't trigger renew operations.
> >> > (I guess this behavior can be somewhat reasonable: maybe the daemons
> >> > try to not trigger DHCP renew on temporary link instability)
> >> >
> >> networkd does not suffer from this problem, and in ifplugd it can be
> > I didn't have time to check the code of networkd, but I think it does have the
> > same behavior.
> > e.g., on a bare metal host with Ubuntu 14.04, when I plug the RJ45 cable out
> > of the network card and then plug the cable back into the network card
> > quickly -- in ~3 seconds, networkd doesn't trigger DHCP renew request: in
> > /var/log/syslog, we see
> > Aug 12 11:07:07 decui-lin NetworkManager[828]: <info> (eth0): carrier now OFF (device state 100, deferring action for 4 seconds)
> > Aug 12 11:07:07 decui-lin kernel: [  246.975453] e1000e: eth0 NIC Link is Down
> > Aug 12 11:07:10 decui-lin NetworkManager[828]: <info> (eth0): carrier now ON (device state 100)
> > Aug 12 11:07:10 decui-lin kernel: [  250.028533] e1000e: eth0 NIC Link is Up 100 Mbps Full Duplex, Flow Control: Rx/Tx
> >
> > It looks there is a delay of 4s.
> > I'm going to find out if there is a configurable parameter for this.
> 
> Just to avoid any confusion: you are referring to "networkd" (and so
> did my comments), but the above logs are from "NetworkManager".

And yes, NM does have a 4-second delay before processing a carrier down
event, and NM currently does not renew DHCP on link changes, but that's
certainly something we can/should change.

Dan

> >> disabled. Most other network drivers will send
> >> IFF_LOWER_DOWN/IFF_LOWER_UP upon suspend/resume so if you were to
> >> do the same you will not work any worse than the others. What would be
> > suspend/resume(ACPI S3?) is different  as this is usually > 4 seconds?  :-)
> 
> Sure, but that's clearly not something we should rely on.
> 
> >> nice, as mentioned by Dan and Lennart, would be to send an additional
> >> explicit event such as "resumed from suspend" or "L3 may be wrong".
> > Sorry, I neglected to reply this.
> > IMHO even if we add the new event, we still need lots of efforts to
> > make the various userland daemons(ifplugd, networkd, etc) to use the
> > new event.
> > And looks we're the first user of this new event. I'm not sure if this issue
> > here can convince the network subsystem maintainers such a new event
> > is a must.
> >
> >> That should be a generic thing though, to fix all such issues in one
> >> go.
> > I agree, though this requires we update all the userland daemons...
> >
> >> > I'm not sure our attempt to "fix" the daemons can be easily accepted.
> >> > BTW, by CPUID, an application has a reliable way to determine  if it's
> >> > running on hyper-v on not. Maybe we can "fix" the behavior of the
> >> > daemons when they run on hyper-v?
> >> > BTW2, according to my limited experience, I doubt other VMMs can
> >> > handle this auto-DHCP-renew-in-guest issue properly.
> >>
> >> To the extent this is a problem, it is a generic one, so we should not
> >> need any hyper-v specific logic in userspace.
> > OK, I understood.
> >
> >> > That was why Yue's patch wanted to add a SLEEP(10s) between the
> >> > link-down and link-up events and hoped this could be an acceptable
> >> > fix(while it turned out not, obviously), though we admit it's not so good
> >> > to add such a magic number "10s" in a kernel driver.
> >> >
> >> > Please point it out if I missed or misunderstand something.
> >> >
> >> > Now I understand it's not good to pass the event to the udev daemon,
> >> > and it's not good to use a SLEEP(10s) in the kernel space(even if it's in a
> >> > "work" task here).
> >>
> >> Please just expose to userspace what is happening (link lost/gained,
> >> resumed from suspend/...), and let us sort out how to react to that.
> >> If you put assumptions about what kind of timeouts (long-dead)
> >> userspace uses into your drivers you'll just create a mess.
> > OK, I got it now.
> > So I think I'm supposed to send out a netif_carrier_off()/on() patch,
> > and I'd better do this after I verify the daemons can work with
> > proper parameters specified.
> >
> >> > Please let me know if it's the correct direction to fix the user-space
> >> > daemons (ifplugd, systemd-networkd, etc).
> >> > If you think this is viable and we should do this, I'll submit a
> >> > netif_carrier_off/on patch first and will start to work with the
> >> > projects of ifplugd, systemd-networkd and many OSVs to make the
> >> > while thing work eventually.
> >>
> >> Have you actually checked that carrier_off/on does not work on
> >> anything but ifplugd? It would surprise me...
> > I can confirm carrier_off/on with 0 delay between the off and on
> > doesn't work for ifplugd and networkd.
> >
> > Thanks,
> > -- Dexuan
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2014-08-13 13:34 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-18 10:55 [PATCH] Hyperv: Trigger DHCP renew after host hibernation Yue Zhang
2014-07-18 10:13 ` Varka Bhadram
2014-07-21  2:45   ` Yue Zhang (OSTC DEV)
2014-07-18 13:40 ` Richard Weinberger
2014-07-21  2:44   ` Yue Zhang (OSTC DEV)
2014-07-21  6:55     ` Richard Weinberger
2014-07-21  8:05       ` Yue Zhang (OSTC DEV)
2014-07-21  8:17         ` Richard Weinberger
2014-07-21  8:44           ` Yue Zhang (OSTC DEV)
2014-07-21  9:01             ` Richard Weinberger
2014-07-21  9:18               ` Olaf Hering
2014-07-21 21:32                 ` David Miller
2014-08-07 22:37                   ` Richard Weinberger
2014-08-08  3:13                     ` Dexuan Cui
2014-08-08  3:32                       ` Greg KH
2014-08-08  8:11                         ` Dexuan Cui
2014-08-08 13:45                           ` Greg KH
2014-08-08 16:28                             ` Stephen Hemminger
2014-08-11  3:23                             ` Dexuan Cui
2014-08-11  3:51                               ` Florian Fainelli
2014-08-11  4:22                                 ` Bill Fink
2014-08-12  7:51                                   ` Dexuan Cui
2014-08-12  7:48                                 ` Dexuan Cui
2014-08-11 10:45                               ` Tom Gundersen
2014-08-12  8:29                                 ` Dexuan Cui
2014-08-13 13:15                                   ` Tom Gundersen
2014-08-13 13:34                                     ` Dan Williams [this message]
2014-08-14  5:19                                       ` Dexuan Cui
2014-08-11  3:29                             ` Dexuan Cui
2014-07-21  9:42 ` Tom Gundersen
2014-07-21 10:21   ` Yue Zhang (OSTC DEV)
2014-07-21 13:11     ` Lennart Poettering
2014-07-21 17:17       ` Dan Williams
2014-07-21 14:06     ` Tom Gundersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1407936888.1825.0.camel@dcbw.local \
    --to=dcbw@redhat.com \
    --cc=davem@davemloft.net \
    --cc=driverdev-devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=huishao@microsoft.com \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olaf@aepfle.de \
    --cc=richard.weinberger@gmail.com \
    --cc=stephen@networkplumber.org \
    --cc=teg@jklm.no \
    --cc=yuezha@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).