| Message ID | 2f001839-2628-cf80-f5e3-415e7492e206@siemens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ipheth: fix EOVERFLOW in ipheth_rcvbulk_callback | expand |
On Mon, 31 Jan 2022 19:58:14 +0100 Jan Kiszka wrote: > From: Georgi Valkov <gvalkov@abv.bg> > > When rx_buf is allocated we need to account for IPHETH_IP_ALIGN, > which reduces the usable size by 2 bytes. Otherwise we have 1512 > bytes usable instead of 1514, and if we receive more than 1512 > bytes, ipheth_rcvbulk_callback is called with status -EOVERFLOW, > after which the driver malfunctiones and all communication stops. > > Resolves ipheth 2-1:4.2: ipheth_rcvbulk_callback: urb status: -75 > > Fixes: f33d9e2b48a3 ("usbnet: ipheth: fix connectivity with iOS 14") > Signed-off-by: Georgi Valkov <gvalkov@abv.bg> > Tested-by: Jan Kiszka <jan.kiszka@siemens.com> Hm, I'm starting to suspect this patch is cursed.. > diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c > index cd33955df0b6..6a769df0b421 100644 > --- a/drivers/net/usb/ipheth.c > +++ b/drivers/net/usb/ipheth.c > @@ -121,7 +121,7 @@ static int ipheth_alloc_urbs(struct ipheth_device *iphone) > if (tx_buf == NULL) There is an extra space character at the start of each line of context. > goto free_rx_urb; > > - rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE, But not on the changed lines. > + rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, > GFP_KERNEL, &rx_urb->transfer_dma); > if (rx_buf == NULL) > goto free_tx_buf; > @@ -146,7 +146,7 @@ static int ipheth_alloc_urbs(struct ipheth_device *iphone) > > static void ipheth_free_urbs(struct ipheth_device *iphone) > { > - usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE, iphone->rx_buf, Pretty clear here in how the opening bracket does not align after the -. > + usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, iphone->rx_buf, > iphone->rx_urb->transfer_dma); > usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE, iphone->tx_buf, > iphone->tx_urb->transfer_dma); > @@ -317,7 +317,7 @@ static int ipheth_rx_submit(struct ipheth_device *dev, gfp_t mem_flags) > > usb_fill_bulk_urb(dev->rx_urb, udev, > usb_rcvbulkpipe(udev, dev->bulk_in), > - dev->rx_buf, IPHETH_BUF_SIZE, > + dev->rx_buf, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, > ipheth_rcvbulk_callback, > dev); > dev->rx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
On 01.02.22 06:14, Jakub Kicinski wrote: > On Mon, 31 Jan 2022 19:58:14 +0100 Jan Kiszka wrote: >> From: Georgi Valkov <gvalkov@abv.bg> >> >> When rx_buf is allocated we need to account for IPHETH_IP_ALIGN, >> which reduces the usable size by 2 bytes. Otherwise we have 1512 >> bytes usable instead of 1514, and if we receive more than 1512 >> bytes, ipheth_rcvbulk_callback is called with status -EOVERFLOW, >> after which the driver malfunctiones and all communication stops. >> >> Resolves ipheth 2-1:4.2: ipheth_rcvbulk_callback: urb status: -75 >> >> Fixes: f33d9e2b48a3 ("usbnet: ipheth: fix connectivity with iOS 14") >> Signed-off-by: Georgi Valkov <gvalkov@abv.bg> >> Tested-by: Jan Kiszka <jan.kiszka@siemens.com> > > Hm, I'm starting to suspect this patch is cursed.. > >> diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c >> index cd33955df0b6..6a769df0b421 100644 >> --- a/drivers/net/usb/ipheth.c >> +++ b/drivers/net/usb/ipheth.c >> @@ -121,7 +121,7 @@ static int ipheth_alloc_urbs(struct ipheth_device *iphone) >> if (tx_buf == NULL) > > There is an extra space character at the start of each line of context. > >> goto free_rx_urb; >> >> - rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE, > > But not on the changed lines. > >> + rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, >> GFP_KERNEL, &rx_urb->transfer_dma); >> if (rx_buf == NULL) >> goto free_tx_buf; >> @@ -146,7 +146,7 @@ static int ipheth_alloc_urbs(struct ipheth_device *iphone) >> >> static void ipheth_free_urbs(struct ipheth_device *iphone) >> { >> - usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE, iphone->rx_buf, > > Pretty clear here in how the opening bracket does not align after the -. > >> + usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, iphone->rx_buf, >> iphone->rx_urb->transfer_dma); >> usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE, iphone->tx_buf, >> iphone->tx_urb->transfer_dma); >> @@ -317,7 +317,7 @@ static int ipheth_rx_submit(struct ipheth_device *dev, gfp_t mem_flags) >> >> usb_fill_bulk_urb(dev->rx_urb, udev, >> usb_rcvbulkpipe(udev, dev->bulk_in), >> - dev->rx_buf, IPHETH_BUF_SIZE, >> + dev->rx_buf, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, >> ipheth_rcvbulk_callback, >> dev); >> dev->rx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; > Sorry, a submission tool regressed here unnoticed (grrrr!). Will resend. Jan
diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c index cd33955df0b6..6a769df0b421 100644 --- a/drivers/net/usb/ipheth.c +++ b/drivers/net/usb/ipheth.c @@ -121,7 +121,7 @@ static int ipheth_alloc_urbs(struct ipheth_device *iphone) if (tx_buf == NULL) goto free_rx_urb; - rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE, + rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, GFP_KERNEL, &rx_urb->transfer_dma); if (rx_buf == NULL) goto free_tx_buf; @@ -146,7 +146,7 @@ static int ipheth_alloc_urbs(struct ipheth_device *iphone) static void ipheth_free_urbs(struct ipheth_device *iphone) { - usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE, iphone->rx_buf, + usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, iphone->rx_buf, iphone->rx_urb->transfer_dma); usb_free_coherent(iphone->udev, IPHETH_BUF_SIZE, iphone->tx_buf, iphone->tx_urb->transfer_dma); @@ -317,7 +317,7 @@ static int ipheth_rx_submit(struct ipheth_device *dev, gfp_t mem_flags) usb_fill_bulk_urb(dev->rx_urb, udev, usb_rcvbulkpipe(udev, dev->bulk_in), - dev->rx_buf, IPHETH_BUF_SIZE, + dev->rx_buf, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN, ipheth_rcvbulk_callback, dev); dev->rx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;