public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Cohen, Eugene" <eugene@hp.com>, "Ye, Ting" <ting.ye@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Fu, Siyuan" <siyuan.fu@intel.com>
Subject: Re: DHCP Automatic Configure at Driver Connect
Date: Thu, 18 Aug 2016 02:48:45 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B7274137C85DE@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <TU4PR84MB0302CC29EA5337CE4DB49E31B4140@TU4PR84MB0302.NAMPRD84.PROD.OUTLOOK.COM>

Eugene,

> > Ip4Config2OnDhcp4SbInstalled() maybe confused you, but it's not
> > accurate as you said "when the DHCP SB is installed you do a configure
> automatically".
> > It's more proper to say "IPv4 driver is requiring DHCP SB due to it
> > detects the DHCP policy." In such a case, it's reasonable with the
> > function Ip4Config2OnDhcp4SbInstalled().
> 
> I see - my observation was based on the fact that removing this prevented the
> undesirable automatic configuration, but what you say makes sense.
> 
> > I agree with you this code did not exist in *Ip4Dxe* before. We
> > implemented this function because of we need start the AutoConfig due
> > to the DHCP policy is detected by Ip4Dxe driver (DHCP policy (note:
> > NOT DHCP SB) together with D.O.R.A process). It does may appear
> > AutoConfig straight away with DHCP SB if Ip4Dxe ahead of Dhcp4Dxe. But
> > the precondition is that the DHCP policy has been detected.  If the
> > policy is not DHCP during the system boot, I think your concern will not
> appear.
> 
> Are you saying the IP4 interface will not come "up" even for a static IP policy?

If the policy is static and the default interface has not been referenced/used, we can say it's not come up.  Once you get involved and do any IP assignment, the default interface is only ready to use, we can say it has been configured and it's available now. 

For *Ip4Dxe*, I think no matter what kind of policy behavior will only affect the default interface (configured or un-configured), currently : 
1) If the policy is DHCP, Ip4Dxe will try his best to be in available state. 
2)  If the policy is static, it will depend on the third part configuration. After the default interface is ready and available, the only way I can think to make it in un-configured state is leveraging the DXE_DRIVER (I mentioned it before) to do some cleanup according the current implementation. 

For #1, the DHCP process will be triggered automatically.
For #2, I agree such a DXE_DRIVER driver will destroy the previous configuration data (Actually, the old Ip4Config behavior also did it), but if not, how can we reach such a un-configured state since the data already saved?


> I was assuming it would in this case.  I'm not sure how big a deal this is because
> if there are no listeners or connections initiated it's kind of a don't-care - I
> think the only difference would be whether responses to ARP are provided.
> 
> > Now, compared to old Ip4Config behavior, we take 'ifconfig' tool
> > command as example - "ifconfig -s eth0 dhcp":
> > The Ip4Config->Start will be invoked to start the auto configuration.
> > It was implemented in the deprecated driver -- Ip4ConfigDxe. When the
> > system boot next time, the previous IP configuration will cleaned and
> > the interface will be in UN-CONFIG status again.  Current
> > implementation don't have this clean-up operation no matter
> > Static/DHCP policy has been set. Is this the difference you mentioned?
> 
> Okay - this must be it - so DHCP magically turned back into "unconfigured"
> effectively creating the on-demand Configure() effect, at least for DHCP cases,
> that we now desire.  Interesting.
> 

Yeah.

> > Now, let us  consider your requirement:
> > 1) The IP config information stored in NVRAM
> > 2) A separate policy to delay the IP interface coming up until a
> > component calls Configure ().
> >
> > #1 is also the current implementation. For 2), I remember you have one
> > patch to do this implementation, can you share it to us for better
> > understanding?
> 
> Yes, my first thought was to make the handling of
> Ip4Config2OnDhcp4SbInstalled conditional based on a PCD value (the policy in
> this case, would be in the PCD value).  Based on the limited experiment I did
> this seems to have the desired effect but based on your expertise perhaps
> there is a better location for such a check?
> 

Ok, understand your purpose. So, It's not related to the IP protocol to be "up" or not , and also not related to *reach un-configured state* if policy is *static*. Right? If so, we can only focus on the question "whether to separate the D.O.R.A process with DHCP policy or not".

In addition, I don't think it's a good idea to use PCD to decide/control the DHCP policy behavior. We should keep the DHCP policy behavior constant (no matter to separate the D.O.R.A process or keep the current implementation).  

In my opinion or usage case, keep the current implementation is fine. But as you complained, it does trouble your previous usage. It will be appreciated if you can describe your usage case here, for example, step1, step2,...:).



> Thank you for patiently helping me with this - it's been enlightening to learn
> the history and thinking behind the config process.
> 


  reply	other threads:[~2016-08-18  2:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-10 18:17 DHCP Automatic Configure at Driver Connect Cohen, Eugene
2016-08-11  1:50 ` Wu, Jiaxin
2016-08-11  3:03 ` Ye, Ting
2016-08-11  5:59   ` Wu, Jiaxin
2016-08-11 14:22     ` Cohen, Eugene
2016-08-11 15:20       ` Samer El Haj Mahmoud
2016-08-12  1:31       ` Wu, Jiaxin
2016-08-12 12:31         ` Cohen, Eugene
2016-08-13  2:37           ` Wu, Jiaxin
2016-08-15 14:40             ` Cohen, Eugene
2016-08-16  3:05               ` Wu, Jiaxin
2016-08-16 19:41                 ` Cohen, Eugene
2016-08-17  5:50                   ` Wu, Jiaxin
2016-08-17 12:17                     ` Cohen, Eugene
2016-08-18  2:48                       ` Wu, Jiaxin [this message]
2016-08-18  3:10                         ` Ye, Ting
2016-08-18 14:30                           ` Cohen, Eugene
2016-08-16  1:14       ` Ye, Ting
2016-08-16 19:19         ` Cohen, Eugene

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-list 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=895558F6EA4E3B41AC93A00D163B7274137C85DE@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /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