public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Cohen, Eugene" <eugene@hp.com>
To: "Wu, Jiaxin" <jiaxin.wu@intel.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>
Subject: Re: DHCP Automatic Configure at Driver Connect
Date: Mon, 15 Aug 2016 14:40:29 +0000	[thread overview]
Message-ID: <AT5PR84MB0291B889682E4F966619DE1DB4120@AT5PR84MB0291.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <895558F6EA4E3B41AC93A00D163B7274137C677D@SHSMSX103.ccr.corp.intel.com>

Jiaxin,

> > > In my memory, we only talked about the performance issue before.
> > > Here, I understand your requirement: 1) The network interface should
> > > be in
> > > *un-
> > > configured* status at each boot time until the third part
> > > configuration;

Yes, we would like a policy setting (either runtime/protocol or build-time/PCD is fine) for this.

 2) the policy setting should be ignored, right? I
> > > don't

I don't know what you mean by this.  The static/dhcp policy shouldn't necessarily be ignored but if the interface is not yet configured (i.e. the IP layer won't respond to packets) then the static/dhcp policy doesn't really matter until some component calls Configure() for the first time.

> This is what I want to confirm with you that my understanding is right? You
> complained the network interface comes "up" at each boot, what does
> "comes up" exactly mean? So, I gave the above two guesses 1) and 2).

And I tried to clarify what I meant.  Another way of saying it is by being "up" it means performing DHCP (if that is the setting) and transmitting/receiving packets using the IP address.  What we are asking for is a policy where we can suppress this behavior until the first Configure() call like before.

> Of course not. I mean if my guessing 2) is correct, it's not reasonable because
> UEFI 2.6 spec only defined Static/DHCP policy. The policy should be in one of
> them. I just recalled the old Ip4Config behavior: no policy assigned default (if
> I remember correctly).  So, don't misunderstand my truly means. It's
> unrelated to the *behavior consistency*.

Perhaps not spec consistency but we need this for implementation consistency since we've built up years of expectations from the previous implementation.

> First, *based on the current UEFI Spec (only static / dhcp policy)*, I want to
> highlight my understanding of *un-configured* status you mentioned: policy
> should be always set, *un-configured* means *no IP address configuration*.

Correct, in that the device will neither transmit or respond at an IP layer initially.

> If we want to implement such a *un-configured* state, one *DXE_DRIVER*
> can be used with Ip4Config2 protocol. Detailed see below pseudocode:
> 
> First,  register a notify for Ip4Config2 protocol in your Dxe Driver EntryPoint:
>     EfiCreateProtocolNotifyEvent (
>         &gEfiIp4Config2ProtocolGuid,
>         TPL_CALLBACK,
>         Ip4Config2InstalledCallback,
>         NULL,
>         &Registration
>         );
> 
> Then, In your Ip4Config2InstalledCallback() function, you can change the
> default policy for the current boot:
>      Ip4Config2InstalledCallback () {
>          gBS->LocateProtocol (
>               &gEfiIp4Config2ProtocolGuid,
>               Registration,
>               (VOID **) &Ip4Config2Instance
>               );
> 
>          //
>          // Change the policy to Ip4Config2PolicyDhcp to clean the static setting.
>         //
>         Policy = Ip4Config2PolicyDhcp;
>         Ip4Config2Instance->SetData(
>                                                         Ip4Config2Instance,
>                                                         Ip4Config2DataTypePolicy,
>                                                         sizeof (EFI_IP4_CONFIG2_POLICY),
>                                                         &Policy
>                                                         );
> 
>          //
>          // Change the policy to Ip4Config2PolicyStatic to clean the DHCP policy
> setting.
>         //
>         Policy = Ip4Config2PolicyStatic;
>         Ip4Config2Instance->SetData(
>                                                         Ip4Config2Instance,
>                                                         Ip4Config2DataTypePolicy,
>                                                         sizeof (EFI_IP4_CONFIG2_POLICY),
>                                                         &Policy
>                                                         );
>     }
> 
> After that, your previous setting will be cleaned, and reach a  *un-
> configured* state. I know the addition Dxe Driver is an indirect way to meet
> your requirement, but based on current UEFI Spec, it's one-time event if you
> added in  your platform ahead.

There's two issues I see with this approach:

1. It destroys any previous configuration data that the user may have made (static IP entry or explicit selection of DHCP)
2. It's a hack where we're using one policy interface to try to accomplish something unrelated (auto vs on-demand config) based on attributes of the current implementation and not the spec

So I'd like to propose again that we define another, separate, policy value for automatic versus on-demand IP configuration.  It can be either through a PCD or a protocol interface.  My preference in the short term is to do the PCD thing since we can do it now where the protocol approach will require spec modification.

Let me know if that makes sense - I'm prepared to provide a patch for the PCD option if/when you're ready.

Eugene




  reply	other threads:[~2016-08-15 14:40 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 [this message]
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
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=AT5PR84MB0291B889682E4F966619DE1DB4120@AT5PR84MB0291.NAMPRD84.PROD.OUTLOOK.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