Julien Viard de Galbert
2017-May-31 07:56 UTC
[syslinux] [PATCH 1/4] efi/udp: core_udp_connect should use SubnetMask not StationAddress for netmask
Signed-off-by: Julien Viard de Galbert <jviarddegalbert at online.net> --- efi/udp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/efi/udp.c b/efi/udp.c index 1088f47..b0f13ad 100644 --- a/efi/udp.c +++ b/efi/udp.c @@ -163,7 +163,7 @@ void core_udp_connect(struct pxe_pvt_inode *socket, uint32_t ip, } else { udata.UseDefaultAddress = FALSE; memcpy(&udata.StationAddress, &IPInfo.myip, sizeof(IPInfo.myip)); - memcpy(&udata.StationAddress, &IPInfo.netmask, sizeof(IPInfo.netmask)); + memcpy(&udata.SubnetMask, &IPInfo.netmask, sizeof(IPInfo.netmask)); } memcpy(&udata.RemoteAddress, &ip, sizeof(ip)); udata.RemotePort = port; -- 2.9.3
Julien Viard de Galbert
2017-May-31 07:56 UTC
[syslinux] [PATCH 2/4] efi/udp: Add retry disabling UseDefaultAddress in core_udp_connect and core_udp_sendto
Signed-off-by: Julien Viard de Galbert <jviarddegalbert at online.net> --- efi/udp.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/efi/udp.c b/efi/udp.c index b0f13ad..8f4d7dc 100644 --- a/efi/udp.c +++ b/efi/udp.c @@ -52,7 +52,7 @@ EFI_STATUS core_udp_configure(EFI_UDP4 *udp, EFI_UDP4_CONFIG_DATA *udata, } } else { if (status != EFI_SUCCESS) { - Print(L"%s: udp->Configure() unsuccessful (%d)", f, status); + Print(L"%s: udp->Configure() unsuccessful (%d)\n", f, status); if (!efi_net_def_addr && (status == EFI_INVALID_PARAMETER)) efi_net_def_addr = 2; } @@ -158,6 +158,7 @@ void core_udp_connect(struct pxe_pvt_inode *socket, uint32_t ip, /* Re-use the existing local port number */ udata.StationPort = socket->net.efi.localport; +retry: if (efi_net_def_addr) { udata.UseDefaultAddress = TRUE; } else { @@ -170,6 +171,11 @@ void core_udp_connect(struct pxe_pvt_inode *socket, uint32_t ip, udata.TimeToLive = 64; status = core_udp_configure(udp, &udata, L"core_udp_connect"); + if (efi_net_def_addr && (status == EFI_NO_MAPPING)) { + efi_net_def_addr = 0; + Print(L"disable UseDefaultAddress\n"); + goto retry; + } if (status != EFI_SUCCESS) { Print(L"Failed to configure UDP: %d\n", status); return; @@ -392,6 +398,7 @@ void core_udp_sendto(struct pxe_pvt_inode *socket, const void *data, /* Re-use the existing local port number */ udata.StationPort = socket->net.efi.localport; +retry: if (efi_net_def_addr) { udata.UseDefaultAddress = TRUE; } else { @@ -404,6 +411,11 @@ void core_udp_sendto(struct pxe_pvt_inode *socket, const void *data, udata.TimeToLive = 64; status = core_udp_configure(udp, &udata, L"core_udp_sendto"); + if (efi_net_def_addr && (status == EFI_NO_MAPPING)) { + efi_net_def_addr = 0; + Print(L"disable UseDefaultAddress\n"); + goto retry; + } if (status != EFI_SUCCESS) goto bail; -- 2.9.3
Julien Viard de Galbert
2017-May-31 07:56 UTC
[syslinux] [PATCH 3/4] Removing the udp_reader efi_binding
This extra socket cause the EFI udp stack to duplicate every packet to be able to provide them to both udp_reader and socket->net.efi.binding. The ones in socket->net.efi.binding were never read so they accumulated in the stack on EFI side making the stack slower and slower. Slow enough to not be able to download a kernel+initramfs before the EFI timeout (5 minutes). Signed-off-by: Julien Viard de Galbert <jviarddegalbert at online.net> --- efi/udp.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/efi/udp.c b/efi/udp.c index 8f4d7dc..4032d0b 100644 --- a/efi/udp.c +++ b/efi/udp.c @@ -10,14 +10,6 @@ extern EFI_GUID Udp4ServiceBindingProtocol, Udp4Protocol; -/* - * This UDP binding is configured to operate in promiscuous mode. It is - * only used for reading packets. It has no associated state unlike - * socket->net.efi.binding, which has a remote IP address and port - * number. - */ -static struct efi_binding *udp_reader; - static int volatile efi_udp_has_recv = 0; int volatile efi_net_def_addr = 1; @@ -76,17 +68,11 @@ int core_udp_open(struct pxe_pvt_inode *socket) EFI_STATUS status; EFI_UDP4 *udp; - (void)socket; - - udp_reader = efi_create_binding(&Udp4ServiceBindingProtocol, &Udp4Protocol); - if (!udp_reader) - return -1; - b = efi_create_binding(&Udp4ServiceBindingProtocol, &Udp4Protocol); if (!b) goto bail; - udp = (EFI_UDP4 *)udp_reader->this; + udp = (EFI_UDP4 *)b->this; memset(&udata, 0, sizeof(udata)); @@ -114,9 +100,6 @@ bail: if (b) efi_destroy_binding(b, &Udp4ServiceBindingProtocol); - efi_destroy_binding(udp_reader, &Udp4ServiceBindingProtocol); - udp_reader = NULL; - return -1; } @@ -127,9 +110,6 @@ bail: */ void core_udp_close(struct pxe_pvt_inode *socket) { - efi_destroy_binding(udp_reader, &Udp4ServiceBindingProtocol); - udp_reader = NULL; - if (!socket->net.efi.binding) return; @@ -239,7 +219,7 @@ int core_udp_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len, (void)socket; - b = udp_reader; + b = socket->net.efi.binding; udp = (EFI_UDP4 *)b->this; memset(&token, 0, sizeof(token)); -- 2.9.3
Julien Viard de Galbert
2017-May-31 07:56 UTC
[syslinux] [PATCH 4/4] Convert time measurement from jiffies to ms_timer
So now the total timeout for 'stalling on configure with no mapping' is 1 sec Signed-off-by: Julien Viard de Galbert <jviarddegalbert at online.net> --- core/fs/pxe/tftp.c | 22 +++++++++++----------- efi/tcp.c | 6 +++--- efi/udp.c | 14 +++++++------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/core/fs/pxe/tftp.c b/core/fs/pxe/tftp.c index 594152b..b507761 100644 --- a/core/fs/pxe/tftp.c +++ b/core/fs/pxe/tftp.c @@ -82,7 +82,7 @@ static void tftp_get_packet(struct inode *inode) uint8_t timeout; uint16_t buffersize; uint16_t serial; - jiffies_t oldtime; + mstime_t oldtime; struct tftp_packet *pkt = NULL; uint16_t buf_len; struct pxe_pvt_inode *socket = PVT(inode); @@ -96,7 +96,7 @@ static void tftp_get_packet(struct inode *inode) */ timeout_ptr = TimeoutTable; timeout = *timeout_ptr++; - oldtime = jiffies(); + oldtime = ms_timer(); ack_again: ack_packet(inode, socket->tftp_lastpkt); @@ -106,7 +106,7 @@ static void tftp_get_packet(struct inode *inode) err = core_udp_recv(socket, socket->tftp_pktbuf, &buf_len, &src_ip, &src_port); if (err) { - jiffies_t now = jiffies(); + mstime_t now = ms_timer(); if (now-oldtime >= timeout) { oldtime = now; @@ -198,8 +198,8 @@ void tftp_open(struct url_info *url, int flags, struct inode *inode, int buffersize; int rrq_len; const uint8_t *timeout_ptr; - jiffies_t timeout; - jiffies_t oldtime; + mstime_t timeout; + mstime_t oldtime; uint16_t opcode; uint16_t blk_num; uint64_t opdata; @@ -241,7 +241,7 @@ sendreq: timeout = *timeout_ptr++; if (!timeout) return; /* No file available... */ - oldtime = jiffies(); + oldtime = ms_timer(); core_udp_sendto(socket, rrq_packet_buf, rrq_len, url->ip, url->port); @@ -253,7 +253,7 @@ wait_pkt: err = core_udp_recv(socket, reply_packet_buf, &buf_len, &src_ip, &src_port); if (err) { - jiffies_t now = jiffies(); + mstime_t now = ms_timer(); if (now - oldtime >= timeout) goto sendreq; } else { @@ -448,8 +448,8 @@ __export int tftp_put(struct url_info *url, int flags, struct inode *inode, int err; int wrq_len; const uint8_t *timeout_ptr; - jiffies_t timeout; - jiffies_t oldtime; + mstime_t timeout; + mstime_t oldtime; uint16_t opcode; uint16_t src_port = url->port; uint32_t src_ip; @@ -493,7 +493,7 @@ sendreq: timeout = *timeout_ptr++; if (!timeout) return return_code; /* No file available... */ - oldtime = jiffies(); + oldtime = ms_timer(); core_udp_sendto(socket, wrq_packet_buf, wrq_len, url->ip, src_port); @@ -504,7 +504,7 @@ sendreq: err = core_udp_recv(socket, reply_packet_buf, &buf_len, &src_ip, &src_port); if (err) { - jiffies_t now = jiffies(); + mstime_t now = ms_timer(); if (now - oldtime >= timeout) goto sendreq; } else { diff --git a/efi/tcp.c b/efi/tcp.c index 0e9140e..b252ad0 100644 --- a/efi/tcp.c +++ b/efi/tcp.c @@ -57,7 +57,7 @@ int core_tcp_connect(struct pxe_pvt_inode *socket, uint32_t ip, uint16_t port) EFI_TCP4 *tcp = (EFI_TCP4 *)b->this; int rv = -1; int unmapped = 1; - jiffies_t start, last, cur; + mstime_t start, last, cur; memset(&tdata, 0, sizeof(tdata)); @@ -69,11 +69,11 @@ int core_tcp_connect(struct pxe_pvt_inode *socket, uint32_t ip, uint16_t port) tdata.TimeToLive = 64; - last = start = jiffies(); + last = start = ms_timer(); while (unmapped){ status = uefi_call_wrapper(tcp->Configure, 2, tcp, &tdata); if (status == EFI_NO_MAPPING) { - cur = jiffies(); + cur = ms_timer(); if ( (cur - last) >= EFI_NOMAP_PRINT_DELAY ) { last = cur; Print(L"core_tcp_connect: stalling on configure with no mapping\n"); diff --git a/efi/udp.c b/efi/udp.c index 4032d0b..d23c884 100644 --- a/efi/udp.c +++ b/efi/udp.c @@ -28,13 +28,13 @@ EFI_STATUS core_udp_configure(EFI_UDP4 *udp, EFI_UDP4_CONFIG_DATA *udata, { EFI_STATUS status; int unmapped = 1; - jiffies_t start, last, cur; + mstime_t start, last, cur; - last = start = jiffies(); + last = start = ms_timer(); while (unmapped){ status = uefi_call_wrapper(udp->Configure, 2, udp, udata); if (status == EFI_NO_MAPPING) { - cur = jiffies(); + cur = ms_timer(); if ( (cur - last) >= EFI_NOMAP_PRINT_DELAY ) { last = cur; Print(L"%s: stalling on configure with no mapping\n", f); @@ -215,7 +215,7 @@ int core_udp_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len, EFI_UDP4 *udp; size_t size; int rv = -1; - jiffies_t start; + mstime_t start; (void)socket; @@ -232,11 +232,11 @@ int core_udp_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len, if (status != EFI_SUCCESS) goto bail; - start = jiffies(); + start = ms_timer(); while (cb_status == -1) { /* 15ms receive timeout... */ - if (jiffies() - start >= 15) { - if (jiffies() - start >= 30) + if (ms_timer() - start >= 15) { + if (ms_timer() - start >= 30) dprintf("Failed to cancel UDP\n"); uefi_call_wrapper(udp->Cancel, 2, udp, &token); -- 2.9.3
Gene Cumm
2017-May-31 10:56 UTC
[syslinux] [PATCH 1/4] efi/udp: core_udp_connect should use SubnetMask not StationAddress for netmask
On Wed, May 31, 2017 at 3:56 AM, Julien Viard de Galbert via Syslinux <syslinux at zytor.com> wrote:> Signed-off-by: Julien Viard de Galbert <jviarddegalbert at online.net> > --- > efi/udp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/efi/udp.c b/efi/udp.c > index 1088f47..b0f13ad 100644 > --- a/efi/udp.c > +++ b/efi/udp.c > @@ -163,7 +163,7 @@ void core_udp_connect(struct pxe_pvt_inode *socket, uint32_t ip, > } else { > udata.UseDefaultAddress = FALSE; > memcpy(&udata.StationAddress, &IPInfo.myip, sizeof(IPInfo.myip)); > - memcpy(&udata.StationAddress, &IPInfo.netmask, sizeof(IPInfo.netmask)); > + memcpy(&udata.SubnetMask, &IPInfo.netmask, sizeof(IPInfo.netmask)); > } > memcpy(&udata.RemoteAddress, &ip, sizeof(ip)); > udata.RemotePort = port;Thanks for catching this. Done. -- -Gene
Gene Cumm
2017-May-31 11:00 UTC
[syslinux] [PATCH 2/4] efi/udp: Add retry disabling UseDefaultAddress in core_udp_connect and core_udp_sendto
On Wed, May 31, 2017 at 3:56 AM, Julien Viard de Galbert via Syslinux <syslinux at zytor.com> wrote:> Signed-off-by: Julien Viard de Galbert <jviarddegalbert at online.net> > --- > efi/udp.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) >Excellent idea to trap on EFI_NO_MAPPING and fall back. Done. -- -Gene
Gene Cumm
2017-May-31 11:45 UTC
[syslinux] [PATCH 3/4] Removing the udp_reader efi_binding
On Wed, May 31, 2017 at 3:56 AM, Julien Viard de Galbert via Syslinux <syslinux at zytor.com> wrote:> This extra socket cause the EFI udp stack to duplicate every packet > to be able to provide them to both udp_reader and socket->net.efi.binding. > The ones in socket->net.efi.binding were never read so they accumulated in > the stack on EFI side making the stack slower and slower. > Slow enough to not be able to download a kernel+initramfs before the EFI > timeout (5 minutes). > > Signed-off-by: Julien Viard de Galbert <jviarddegalbert at online.net> > --- > efi/udp.c | 24 ++---------------------- > 1 file changed, 2 insertions(+), 22 deletions(-)That almost certainly explains the decaying download rates.> extern EFI_GUID Udp4ServiceBindingProtocol, Udp4Protocol; > > -/* > - * This UDP binding is configured to operate in promiscuous mode. It is > - * only used for reading packets. It has no associated state unlike > - * socket->net.efi.binding, which has a remote IP address and port > - * number. > - */This part has me puzzled why Matt was creating a promiscuous binding instead of re-using the existing binding. -- -Gene
Apparently Analagous Threads
- [PATCH 3/4] Removing the udp_reader efi_binding
- "Tick-counting" vs "Tick-less" timekeeping issues on VMs emulating BIOS PCs
- [PATCH 3/4] Removing the udp_reader efi_binding
- [PATCH 2/5] ntfs: remove unused variable and have ntfssect use char API calls
- [PATCH 2/4] efi/udp: Add retry disabling UseDefaultAddress in core_udp_connect and core_udp_sendto