| Message ID | 20200207091017.26244-9-greearb@candelatech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/9] supplicant: Update HS20 readme. | expand |
On Fri, Feb 07, 2020 at 01:10:17AM -0800, greearb@candelatech.com wrote: > For instance, setting up VRFs and such might take a small bit of extra time. What is VRF and why would a user space tool on a station device need to know about that? > diff --git a/hs20/client/oma_dm_client.c b/hs20/client/oma_dm_client.c > @@ -1211,6 +1211,8 @@ int cmd_oma_dm_sim_prov(struct hs20_osu_client *ctx, const char *url) > if (wait_ip_addr(ctx->ifname, 15) < 0) { > wpa_printf(MSG_INFO, "Could not get IP address for WLAN - try connection anyway"); > } > + /* Give a bit more time in case external tools are still configuring things, like VRF. */ > + os_sleep(1, 0); > write_summary(ctx, "OMA-DM SIM provisioning"); Wouldn't it make more sense to do whatever is needed within wait_ip_addr() instead of copy pasting this into every location where wait_ip_addr() is called? That said, I'm not convinced waiting one second is the thing that should happen here. What exactly are the cases where you have needed to wait longer after the IP address has been assigned? Maybe wait_ip_addr() (or wpa_supplicant before indicating STATUS command ip_address field) should wait for those exact things rather than adding a hardcoded wait for one second.
On 02/16/2020 06:33 AM, Jouni Malinen wrote: > On Fri, Feb 07, 2020 at 01:10:17AM -0800, greearb@candelatech.com wrote: >> For instance, setting up VRFs and such might take a small bit of extra time. > > What is VRF and why would a user space tool on a station device need to > know about that? VRF is 'virtual router function', or something like that. It is somewhat similar in use to network name spaces and routing table rules. It allows multiple wlans to route to the same AP, for instance. > >> diff --git a/hs20/client/oma_dm_client.c b/hs20/client/oma_dm_client.c >> @@ -1211,6 +1211,8 @@ int cmd_oma_dm_sim_prov(struct hs20_osu_client *ctx, const char *url) >> if (wait_ip_addr(ctx->ifname, 15) < 0) { >> wpa_printf(MSG_INFO, "Could not get IP address for WLAN - try connection anyway"); >> } >> + /* Give a bit more time in case external tools are still configuring things, like VRF. */ >> + os_sleep(1, 0); >> write_summary(ctx, "OMA-DM SIM provisioning"); > > Wouldn't it make more sense to do whatever is needed within > wait_ip_addr() instead of copy pasting this into every location where > wait_ip_addr() is called? That said, I'm not convinced waiting one > second is the thing that should happen here. > > What exactly are the cases where you have needed to wait longer after > the IP address has been assigned? Maybe wait_ip_addr() (or > wpa_supplicant before indicating STATUS command ip_address field) should > wait for those exact things rather than adding a hardcoded wait for one > second. My testing tool uses a custom DHCP script and sets the IP address, route, and DNS per interface. Part of this also optionally sets up VRF or routing table rules to allow many interfaces to talk to the same AP properly. Since I want to have the OSU client use the DNS given out by the AP (and not whatever eth0 management face is already using), then that is why I want to have the separate file option to hold the DNS entry per wlan. In testing, I saw cases where connectivity to the hs20 web servers timed out some of the time. I thought maybe it was a race in setting up the vrf, but in truth, I am not sure that is the root cause. Probably no one else needs this particular behaviour, so you can drop that 1 sec sleep patch and probably that works fine for everyone else. Since normal frameworks are probably not going to auto-generate the DNS file or set up fancy routing rules, it is probably not worth mucking with the wait-for-ip logic. Thanks, Ben
diff --git a/hs20/client/oma_dm_client.c b/hs20/client/oma_dm_client.c index 78b46341a..ad5724525 100644 --- a/hs20/client/oma_dm_client.c +++ b/hs20/client/oma_dm_client.c @@ -1211,6 +1211,8 @@ int cmd_oma_dm_sim_prov(struct hs20_osu_client *ctx, const char *url) if (wait_ip_addr(ctx->ifname, 15) < 0) { wpa_printf(MSG_INFO, "Could not get IP address for WLAN - try connection anyway"); } + /* Give a bit more time in case external tools are still configuring things, like VRF. */ + os_sleep(1, 0); write_summary(ctx, "OMA-DM SIM provisioning"); check_dns_file(ctx); diff --git a/hs20/client/osu_client.c b/hs20/client/osu_client.c index ff6e5b1c0..3b579f14f 100644 --- a/hs20/client/osu_client.c +++ b/hs20/client/osu_client.c @@ -2285,6 +2285,8 @@ static int osu_connect(struct hs20_osu_client *ctx, const char *bssid, wpa_printf(MSG_INFO, "Could not get IP address for WLAN - try connection anyway"); } + /* Give a bit more time in case external tools are still configuring things, like VRF. */ + os_sleep(1, 0); check_dns_file(ctx); if (no_prod_assoc) { @@ -2730,6 +2732,9 @@ static int cmd_sub_rem(struct hs20_osu_client *ctx, const char *address, if (wait_ip_addr(ctx->ifname, 15) < 0) { wpa_printf(MSG_INFO, "Could not get IP address for WLAN - try connection anyway"); } + + /* Give a bit more time in case external tools are still configuring things, like VRF. */ + os_sleep(1, 0); check_dns_file(ctx); if (spp) diff --git a/hs20/client/spp_client.c b/hs20/client/spp_client.c index 4144189b0..48a8f6457 100644 --- a/hs20/client/spp_client.c +++ b/hs20/client/spp_client.c @@ -1000,6 +1000,8 @@ int cmd_sim_prov(struct hs20_osu_client *ctx, const char *url) wpa_printf(MSG_INFO, "Could not get IP address for WLAN - try connection anyway"); } + /* Give a bit more time in case external tools are still configuring things, like VRF. */ + os_sleep(1, 0); check_dns_file(ctx); if (soap_init_client(ctx->http, url, ctx->ca_fname, NULL, NULL, NULL,