* Re: [PATCH] wireless/p54: Remove duplicated net2280 header
From: Ricardo Ribalda Delgado @ 2014-12-01 10:46 UTC (permalink / raw)
To: Christian Lamparter, John W. Linville, LKML, linux-wireless,
netdev, David Miller
Cc: Ricardo Ribalda Delgado
In-Reply-To: <1416824391-13976-1-git-send-email-ricardo.ribalda@gmail.com>
David Miller has marked the patch as "Awaiting Upstream", which I
think means that it should be merged through the wireless tree.
Any comment from there?
On Mon, Nov 24, 2014 at 11:19 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> The usb gadget driver net2280 has exported a header file with the
> register definition of the net2280 chip.
>
> Remove the custom/duplicated header file in favor of that header file
> in include/linux
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/net/wireless/p54/net2280.h | 451 -------------------------------------
> drivers/net/wireless/p54/p54usb.h | 13 +-
> 2 files changed, 12 insertions(+), 452 deletions(-)
> delete mode 100644 drivers/net/wireless/p54/net2280.h
>
> diff --git a/drivers/net/wireless/p54/net2280.h b/drivers/net/wireless/p54/net2280.h
> deleted file mode 100644
> index aedfaf2..0000000
> --- a/drivers/net/wireless/p54/net2280.h
> +++ /dev/null
> @@ -1,451 +0,0 @@
> -#ifndef NET2280_H
> -#define NET2280_H
> -/*
> - * NetChip 2280 high/full speed USB device controller.
> - * Unlike many such controllers, this one talks PCI.
> - */
> -
> -/*
> - * Copyright (C) 2002 NetChip Technology, Inc. (http://www.netchip.com)
> - * Copyright (C) 2003 David Brownell
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -/*-------------------------------------------------------------------------*/
> -
> -/* NET2280 MEMORY MAPPED REGISTERS
> - *
> - * The register layout came from the chip documentation, and the bit
> - * number definitions were extracted from chip specification.
> - *
> - * Use the shift operator ('<<') to build bit masks, with readl/writel
> - * to access the registers through PCI.
> - */
> -
> -/* main registers, BAR0 + 0x0000 */
> -struct net2280_regs {
> - /* offset 0x0000 */
> - __le32 devinit;
> -#define LOCAL_CLOCK_FREQUENCY 8
> -#define FORCE_PCI_RESET 7
> -#define PCI_ID 6
> -#define PCI_ENABLE 5
> -#define FIFO_SOFT_RESET 4
> -#define CFG_SOFT_RESET 3
> -#define PCI_SOFT_RESET 2
> -#define USB_SOFT_RESET 1
> -#define M8051_RESET 0
> - __le32 eectl;
> -#define EEPROM_ADDRESS_WIDTH 23
> -#define EEPROM_CHIP_SELECT_ACTIVE 22
> -#define EEPROM_PRESENT 21
> -#define EEPROM_VALID 20
> -#define EEPROM_BUSY 19
> -#define EEPROM_CHIP_SELECT_ENABLE 18
> -#define EEPROM_BYTE_READ_START 17
> -#define EEPROM_BYTE_WRITE_START 16
> -#define EEPROM_READ_DATA 8
> -#define EEPROM_WRITE_DATA 0
> - __le32 eeclkfreq;
> - u32 _unused0;
> - /* offset 0x0010 */
> -
> - __le32 pciirqenb0; /* interrupt PCI master ... */
> -#define SETUP_PACKET_INTERRUPT_ENABLE 7
> -#define ENDPOINT_F_INTERRUPT_ENABLE 6
> -#define ENDPOINT_E_INTERRUPT_ENABLE 5
> -#define ENDPOINT_D_INTERRUPT_ENABLE 4
> -#define ENDPOINT_C_INTERRUPT_ENABLE 3
> -#define ENDPOINT_B_INTERRUPT_ENABLE 2
> -#define ENDPOINT_A_INTERRUPT_ENABLE 1
> -#define ENDPOINT_0_INTERRUPT_ENABLE 0
> - __le32 pciirqenb1;
> -#define PCI_INTERRUPT_ENABLE 31
> -#define POWER_STATE_CHANGE_INTERRUPT_ENABLE 27
> -#define PCI_ARBITER_TIMEOUT_INTERRUPT_ENABLE 26
> -#define PCI_PARITY_ERROR_INTERRUPT_ENABLE 25
> -#define PCI_MASTER_ABORT_RECEIVED_INTERRUPT_ENABLE 20
> -#define PCI_TARGET_ABORT_RECEIVED_INTERRUPT_ENABLE 19
> -#define PCI_TARGET_ABORT_ASSERTED_INTERRUPT_ENABLE 18
> -#define PCI_RETRY_ABORT_INTERRUPT_ENABLE 17
> -#define PCI_MASTER_CYCLE_DONE_INTERRUPT_ENABLE 16
> -#define GPIO_INTERRUPT_ENABLE 13
> -#define DMA_D_INTERRUPT_ENABLE 12
> -#define DMA_C_INTERRUPT_ENABLE 11
> -#define DMA_B_INTERRUPT_ENABLE 10
> -#define DMA_A_INTERRUPT_ENABLE 9
> -#define EEPROM_DONE_INTERRUPT_ENABLE 8
> -#define VBUS_INTERRUPT_ENABLE 7
> -#define CONTROL_STATUS_INTERRUPT_ENABLE 6
> -#define ROOT_PORT_RESET_INTERRUPT_ENABLE 4
> -#define SUSPEND_REQUEST_INTERRUPT_ENABLE 3
> -#define SUSPEND_REQUEST_CHANGE_INTERRUPT_ENABLE 2
> -#define RESUME_INTERRUPT_ENABLE 1
> -#define SOF_INTERRUPT_ENABLE 0
> - __le32 cpu_irqenb0; /* ... or onboard 8051 */
> -#define SETUP_PACKET_INTERRUPT_ENABLE 7
> -#define ENDPOINT_F_INTERRUPT_ENABLE 6
> -#define ENDPOINT_E_INTERRUPT_ENABLE 5
> -#define ENDPOINT_D_INTERRUPT_ENABLE 4
> -#define ENDPOINT_C_INTERRUPT_ENABLE 3
> -#define ENDPOINT_B_INTERRUPT_ENABLE 2
> -#define ENDPOINT_A_INTERRUPT_ENABLE 1
> -#define ENDPOINT_0_INTERRUPT_ENABLE 0
> - __le32 cpu_irqenb1;
> -#define CPU_INTERRUPT_ENABLE 31
> -#define POWER_STATE_CHANGE_INTERRUPT_ENABLE 27
> -#define PCI_ARBITER_TIMEOUT_INTERRUPT_ENABLE 26
> -#define PCI_PARITY_ERROR_INTERRUPT_ENABLE 25
> -#define PCI_INTA_INTERRUPT_ENABLE 24
> -#define PCI_PME_INTERRUPT_ENABLE 23
> -#define PCI_SERR_INTERRUPT_ENABLE 22
> -#define PCI_PERR_INTERRUPT_ENABLE 21
> -#define PCI_MASTER_ABORT_RECEIVED_INTERRUPT_ENABLE 20
> -#define PCI_TARGET_ABORT_RECEIVED_INTERRUPT_ENABLE 19
> -#define PCI_RETRY_ABORT_INTERRUPT_ENABLE 17
> -#define PCI_MASTER_CYCLE_DONE_INTERRUPT_ENABLE 16
> -#define GPIO_INTERRUPT_ENABLE 13
> -#define DMA_D_INTERRUPT_ENABLE 12
> -#define DMA_C_INTERRUPT_ENABLE 11
> -#define DMA_B_INTERRUPT_ENABLE 10
> -#define DMA_A_INTERRUPT_ENABLE 9
> -#define EEPROM_DONE_INTERRUPT_ENABLE 8
> -#define VBUS_INTERRUPT_ENABLE 7
> -#define CONTROL_STATUS_INTERRUPT_ENABLE 6
> -#define ROOT_PORT_RESET_INTERRUPT_ENABLE 4
> -#define SUSPEND_REQUEST_INTERRUPT_ENABLE 3
> -#define SUSPEND_REQUEST_CHANGE_INTERRUPT_ENABLE 2
> -#define RESUME_INTERRUPT_ENABLE 1
> -#define SOF_INTERRUPT_ENABLE 0
> -
> - /* offset 0x0020 */
> - u32 _unused1;
> - __le32 usbirqenb1;
> -#define USB_INTERRUPT_ENABLE 31
> -#define POWER_STATE_CHANGE_INTERRUPT_ENABLE 27
> -#define PCI_ARBITER_TIMEOUT_INTERRUPT_ENABLE 26
> -#define PCI_PARITY_ERROR_INTERRUPT_ENABLE 25
> -#define PCI_INTA_INTERRUPT_ENABLE 24
> -#define PCI_PME_INTERRUPT_ENABLE 23
> -#define PCI_SERR_INTERRUPT_ENABLE 22
> -#define PCI_PERR_INTERRUPT_ENABLE 21
> -#define PCI_MASTER_ABORT_RECEIVED_INTERRUPT_ENABLE 20
> -#define PCI_TARGET_ABORT_RECEIVED_INTERRUPT_ENABLE 19
> -#define PCI_RETRY_ABORT_INTERRUPT_ENABLE 17
> -#define PCI_MASTER_CYCLE_DONE_INTERRUPT_ENABLE 16
> -#define GPIO_INTERRUPT_ENABLE 13
> -#define DMA_D_INTERRUPT_ENABLE 12
> -#define DMA_C_INTERRUPT_ENABLE 11
> -#define DMA_B_INTERRUPT_ENABLE 10
> -#define DMA_A_INTERRUPT_ENABLE 9
> -#define EEPROM_DONE_INTERRUPT_ENABLE 8
> -#define VBUS_INTERRUPT_ENABLE 7
> -#define CONTROL_STATUS_INTERRUPT_ENABLE 6
> -#define ROOT_PORT_RESET_INTERRUPT_ENABLE 4
> -#define SUSPEND_REQUEST_INTERRUPT_ENABLE 3
> -#define SUSPEND_REQUEST_CHANGE_INTERRUPT_ENABLE 2
> -#define RESUME_INTERRUPT_ENABLE 1
> -#define SOF_INTERRUPT_ENABLE 0
> - __le32 irqstat0;
> -#define INTA_ASSERTED 12
> -#define SETUP_PACKET_INTERRUPT 7
> -#define ENDPOINT_F_INTERRUPT 6
> -#define ENDPOINT_E_INTERRUPT 5
> -#define ENDPOINT_D_INTERRUPT 4
> -#define ENDPOINT_C_INTERRUPT 3
> -#define ENDPOINT_B_INTERRUPT 2
> -#define ENDPOINT_A_INTERRUPT 1
> -#define ENDPOINT_0_INTERRUPT 0
> - __le32 irqstat1;
> -#define POWER_STATE_CHANGE_INTERRUPT 27
> -#define PCI_ARBITER_TIMEOUT_INTERRUPT 26
> -#define PCI_PARITY_ERROR_INTERRUPT 25
> -#define PCI_INTA_INTERRUPT 24
> -#define PCI_PME_INTERRUPT 23
> -#define PCI_SERR_INTERRUPT 22
> -#define PCI_PERR_INTERRUPT 21
> -#define PCI_MASTER_ABORT_RECEIVED_INTERRUPT 20
> -#define PCI_TARGET_ABORT_RECEIVED_INTERRUPT 19
> -#define PCI_RETRY_ABORT_INTERRUPT 17
> -#define PCI_MASTER_CYCLE_DONE_INTERRUPT 16
> -#define GPIO_INTERRUPT 13
> -#define DMA_D_INTERRUPT 12
> -#define DMA_C_INTERRUPT 11
> -#define DMA_B_INTERRUPT 10
> -#define DMA_A_INTERRUPT 9
> -#define EEPROM_DONE_INTERRUPT 8
> -#define VBUS_INTERRUPT 7
> -#define CONTROL_STATUS_INTERRUPT 6
> -#define ROOT_PORT_RESET_INTERRUPT 4
> -#define SUSPEND_REQUEST_INTERRUPT 3
> -#define SUSPEND_REQUEST_CHANGE_INTERRUPT 2
> -#define RESUME_INTERRUPT 1
> -#define SOF_INTERRUPT 0
> - /* offset 0x0030 */
> - __le32 idxaddr;
> - __le32 idxdata;
> - __le32 fifoctl;
> -#define PCI_BASE2_RANGE 16
> -#define IGNORE_FIFO_AVAILABILITY 3
> -#define PCI_BASE2_SELECT 2
> -#define FIFO_CONFIGURATION_SELECT 0
> - u32 _unused2;
> - /* offset 0x0040 */
> - __le32 memaddr;
> -#define START 28
> -#define DIRECTION 27
> -#define FIFO_DIAGNOSTIC_SELECT 24
> -#define MEMORY_ADDRESS 0
> - __le32 memdata0;
> - __le32 memdata1;
> - u32 _unused3;
> - /* offset 0x0050 */
> - __le32 gpioctl;
> -#define GPIO3_LED_SELECT 12
> -#define GPIO3_INTERRUPT_ENABLE 11
> -#define GPIO2_INTERRUPT_ENABLE 10
> -#define GPIO1_INTERRUPT_ENABLE 9
> -#define GPIO0_INTERRUPT_ENABLE 8
> -#define GPIO3_OUTPUT_ENABLE 7
> -#define GPIO2_OUTPUT_ENABLE 6
> -#define GPIO1_OUTPUT_ENABLE 5
> -#define GPIO0_OUTPUT_ENABLE 4
> -#define GPIO3_DATA 3
> -#define GPIO2_DATA 2
> -#define GPIO1_DATA 1
> -#define GPIO0_DATA 0
> - __le32 gpiostat;
> -#define GPIO3_INTERRUPT 3
> -#define GPIO2_INTERRUPT 2
> -#define GPIO1_INTERRUPT 1
> -#define GPIO0_INTERRUPT 0
> -} __packed;
> -
> -/* usb control, BAR0 + 0x0080 */
> -struct net2280_usb_regs {
> - /* offset 0x0080 */
> - __le32 stdrsp;
> -#define STALL_UNSUPPORTED_REQUESTS 31
> -#define SET_TEST_MODE 16
> -#define GET_OTHER_SPEED_CONFIGURATION 15
> -#define GET_DEVICE_QUALIFIER 14
> -#define SET_ADDRESS 13
> -#define ENDPOINT_SET_CLEAR_HALT 12
> -#define DEVICE_SET_CLEAR_DEVICE_REMOTE_WAKEUP 11
> -#define GET_STRING_DESCRIPTOR_2 10
> -#define GET_STRING_DESCRIPTOR_1 9
> -#define GET_STRING_DESCRIPTOR_0 8
> -#define GET_SET_INTERFACE 6
> -#define GET_SET_CONFIGURATION 5
> -#define GET_CONFIGURATION_DESCRIPTOR 4
> -#define GET_DEVICE_DESCRIPTOR 3
> -#define GET_ENDPOINT_STATUS 2
> -#define GET_INTERFACE_STATUS 1
> -#define GET_DEVICE_STATUS 0
> - __le32 prodvendid;
> -#define PRODUCT_ID 16
> -#define VENDOR_ID 0
> - __le32 relnum;
> - __le32 usbctl;
> -#define SERIAL_NUMBER_INDEX 16
> -#define PRODUCT_ID_STRING_ENABLE 13
> -#define VENDOR_ID_STRING_ENABLE 12
> -#define USB_ROOT_PORT_WAKEUP_ENABLE 11
> -#define VBUS_PIN 10
> -#define TIMED_DISCONNECT 9
> -#define SUSPEND_IMMEDIATELY 7
> -#define SELF_POWERED_USB_DEVICE 6
> -#define REMOTE_WAKEUP_SUPPORT 5
> -#define PME_POLARITY 4
> -#define USB_DETECT_ENABLE 3
> -#define PME_WAKEUP_ENABLE 2
> -#define DEVICE_REMOTE_WAKEUP_ENABLE 1
> -#define SELF_POWERED_STATUS 0
> - /* offset 0x0090 */
> - __le32 usbstat;
> -#define HIGH_SPEED 7
> -#define FULL_SPEED 6
> -#define GENERATE_RESUME 5
> -#define GENERATE_DEVICE_REMOTE_WAKEUP 4
> - __le32 xcvrdiag;
> -#define FORCE_HIGH_SPEED_MODE 31
> -#define FORCE_FULL_SPEED_MODE 30
> -#define USB_TEST_MODE 24
> -#define LINE_STATE 16
> -#define TRANSCEIVER_OPERATION_MODE 2
> -#define TRANSCEIVER_SELECT 1
> -#define TERMINATION_SELECT 0
> - __le32 setup0123;
> - __le32 setup4567;
> - /* offset 0x0090 */
> - u32 _unused0;
> - __le32 ouraddr;
> -#define FORCE_IMMEDIATE 7
> -#define OUR_USB_ADDRESS 0
> - __le32 ourconfig;
> -} __packed;
> -
> -/* pci control, BAR0 + 0x0100 */
> -struct net2280_pci_regs {
> - /* offset 0x0100 */
> - __le32 pcimstctl;
> -#define PCI_ARBITER_PARK_SELECT 13
> -#define PCI_MULTI LEVEL_ARBITER 12
> -#define PCI_RETRY_ABORT_ENABLE 11
> -#define DMA_MEMORY_WRITE_AND_INVALIDATE_ENABLE 10
> -#define DMA_READ_MULTIPLE_ENABLE 9
> -#define DMA_READ_LINE_ENABLE 8
> -#define PCI_MASTER_COMMAND_SELECT 6
> -#define MEM_READ_OR_WRITE 0
> -#define IO_READ_OR_WRITE 1
> -#define CFG_READ_OR_WRITE 2
> -#define PCI_MASTER_START 5
> -#define PCI_MASTER_READ_WRITE 4
> -#define PCI_MASTER_WRITE 0
> -#define PCI_MASTER_READ 1
> -#define PCI_MASTER_BYTE_WRITE_ENABLES 0
> - __le32 pcimstaddr;
> - __le32 pcimstdata;
> - __le32 pcimststat;
> -#define PCI_ARBITER_CLEAR 2
> -#define PCI_EXTERNAL_ARBITER 1
> -#define PCI_HOST_MODE 0
> -} __packed;
> -
> -/* dma control, BAR0 + 0x0180 ... array of four structs like this,
> - * for channels 0..3. see also struct net2280_dma: descriptor
> - * that can be loaded into some of these registers.
> - */
> -struct net2280_dma_regs { /* [11.7] */
> - /* offset 0x0180, 0x01a0, 0x01c0, 0x01e0, */
> - __le32 dmactl;
> -#define DMA_SCATTER_GATHER_DONE_INTERRUPT_ENABLE 25
> -#define DMA_CLEAR_COUNT_ENABLE 21
> -#define DESCRIPTOR_POLLING_RATE 19
> -#define POLL_CONTINUOUS 0
> -#define POLL_1_USEC 1
> -#define POLL_100_USEC 2
> -#define POLL_1_MSEC 3
> -#define DMA_VALID_BIT_POLLING_ENABLE 18
> -#define DMA_VALID_BIT_ENABLE 17
> -#define DMA_SCATTER_GATHER_ENABLE 16
> -#define DMA_OUT_AUTO_START_ENABLE 4
> -#define DMA_PREEMPT_ENABLE 3
> -#define DMA_FIFO_VALIDATE 2
> -#define DMA_ENABLE 1
> -#define DMA_ADDRESS_HOLD 0
> - __le32 dmastat;
> -#define DMA_SCATTER_GATHER_DONE_INTERRUPT 25
> -#define DMA_TRANSACTION_DONE_INTERRUPT 24
> -#define DMA_ABORT 1
> -#define DMA_START 0
> - u32 _unused0[2];
> - /* offset 0x0190, 0x01b0, 0x01d0, 0x01f0, */
> - __le32 dmacount;
> -#define VALID_BIT 31
> -#define DMA_DIRECTION 30
> -#define DMA_DONE_INTERRUPT_ENABLE 29
> -#define END_OF_CHAIN 28
> -#define DMA_BYTE_COUNT_MASK ((1<<24)-1)
> -#define DMA_BYTE_COUNT 0
> - __le32 dmaaddr;
> - __le32 dmadesc;
> - u32 _unused1;
> -} __packed;
> -
> -/* dedicated endpoint registers, BAR0 + 0x0200 */
> -
> -struct net2280_dep_regs { /* [11.8] */
> - /* offset 0x0200, 0x0210, 0x220, 0x230, 0x240 */
> - __le32 dep_cfg;
> - /* offset 0x0204, 0x0214, 0x224, 0x234, 0x244 */
> - __le32 dep_rsp;
> - u32 _unused[2];
> -} __packed;
> -
> -/* configurable endpoint registers, BAR0 + 0x0300 ... array of seven structs
> - * like this, for ep0 then the configurable endpoints A..F
> - * ep0 reserved for control; E and F have only 64 bytes of fifo
> - */
> -struct net2280_ep_regs { /* [11.9] */
> - /* offset 0x0300, 0x0320, 0x0340, 0x0360, 0x0380, 0x03a0, 0x03c0 */
> - __le32 ep_cfg;
> -#define ENDPOINT_BYTE_COUNT 16
> -#define ENDPOINT_ENABLE 10
> -#define ENDPOINT_TYPE 8
> -#define ENDPOINT_DIRECTION 7
> -#define ENDPOINT_NUMBER 0
> - __le32 ep_rsp;
> -#define SET_NAK_OUT_PACKETS 15
> -#define SET_EP_HIDE_STATUS_PHASE 14
> -#define SET_EP_FORCE_CRC_ERROR 13
> -#define SET_INTERRUPT_MODE 12
> -#define SET_CONTROL_STATUS_PHASE_HANDSHAKE 11
> -#define SET_NAK_OUT_PACKETS_MODE 10
> -#define SET_ENDPOINT_TOGGLE 9
> -#define SET_ENDPOINT_HALT 8
> -#define CLEAR_NAK_OUT_PACKETS 7
> -#define CLEAR_EP_HIDE_STATUS_PHASE 6
> -#define CLEAR_EP_FORCE_CRC_ERROR 5
> -#define CLEAR_INTERRUPT_MODE 4
> -#define CLEAR_CONTROL_STATUS_PHASE_HANDSHAKE 3
> -#define CLEAR_NAK_OUT_PACKETS_MODE 2
> -#define CLEAR_ENDPOINT_TOGGLE 1
> -#define CLEAR_ENDPOINT_HALT 0
> - __le32 ep_irqenb;
> -#define SHORT_PACKET_OUT_DONE_INTERRUPT_ENABLE 6
> -#define SHORT_PACKET_TRANSFERRED_INTERRUPT_ENABLE 5
> -#define DATA_PACKET_RECEIVED_INTERRUPT_ENABLE 3
> -#define DATA_PACKET_TRANSMITTED_INTERRUPT_ENABLE 2
> -#define DATA_OUT_PING_TOKEN_INTERRUPT_ENABLE 1
> -#define DATA_IN_TOKEN_INTERRUPT_ENABLE 0
> - __le32 ep_stat;
> -#define FIFO_VALID_COUNT 24
> -#define HIGH_BANDWIDTH_OUT_TRANSACTION_PID 22
> -#define TIMEOUT 21
> -#define USB_STALL_SENT 20
> -#define USB_IN_NAK_SENT 19
> -#define USB_IN_ACK_RCVD 18
> -#define USB_OUT_PING_NAK_SENT 17
> -#define USB_OUT_ACK_SENT 16
> -#define FIFO_OVERFLOW 13
> -#define FIFO_UNDERFLOW 12
> -#define FIFO_FULL 11
> -#define FIFO_EMPTY 10
> -#define FIFO_FLUSH 9
> -#define SHORT_PACKET_OUT_DONE_INTERRUPT 6
> -#define SHORT_PACKET_TRANSFERRED_INTERRUPT 5
> -#define NAK_OUT_PACKETS 4
> -#define DATA_PACKET_RECEIVED_INTERRUPT 3
> -#define DATA_PACKET_TRANSMITTED_INTERRUPT 2
> -#define DATA_OUT_PING_TOKEN_INTERRUPT 1
> -#define DATA_IN_TOKEN_INTERRUPT 0
> - /* offset 0x0310, 0x0330, 0x0350, 0x0370, 0x0390, 0x03b0, 0x03d0 */
> - __le32 ep_avail;
> - __le32 ep_data;
> - u32 _unused0[2];
> -} __packed;
> -
> -struct net2280_reg_write {
> - __le16 port;
> - __le32 addr;
> - __le32 val;
> -} __packed;
> -
> -struct net2280_reg_read {
> - __le16 port;
> - __le32 addr;
> -} __packed;
> -#endif /* NET2280_H */
> diff --git a/drivers/net/wireless/p54/p54usb.h b/drivers/net/wireless/p54/p54usb.h
> index d273be7..a5f5f0f 100644
> --- a/drivers/net/wireless/p54/p54usb.h
> +++ b/drivers/net/wireless/p54/p54usb.h
> @@ -16,7 +16,7 @@
>
> /* for isl3886 register definitions used on ver 1 devices */
> #include "p54pci.h"
> -#include "net2280.h"
> +#include <linux/usb/net2280.h>
>
> /* pci */
> #define NET2280_BASE 0x10000000
> @@ -93,6 +93,17 @@ enum net2280_op_type {
> NET2280_DEV_CFG_U16 = 0x0883
> };
>
> +struct net2280_reg_write {
> + __le16 port;
> + __le32 addr;
> + __le32 val;
> +} __packed;
> +
> +struct net2280_reg_read {
> + __le16 port;
> + __le32 addr;
> +} __packed;
> +
> #define P54U_FW_BLOCK 2048
>
> #define X2_SIGNATURE "x2 "
> --
> 2.1.3
>
--
Ricardo Ribalda
^ permalink raw reply
* Re: [PATCH RFC v3 0/3] virtio_net: enabling tx interrupts
From: Michael S. Tsirkin @ 2014-12-01 10:48 UTC (permalink / raw)
To: Jason Wang; +Cc: linux-kernel, netdev
In-Reply-To: <547C3F8C.8000006@redhat.com>
On Mon, Dec 01, 2014 at 06:14:36PM +0800, Jason Wang wrote:
>
>
> On 10/20/2014 02:52 PM, Michael S. Tsirkin wrote:
> >RFC patches to enable tx interrupts.
> >This is to demonstrate how this can be done without
> >core virtio changes, and to make sure I understand
> >the new APIs correctly.
> >
> >Testing TBD, I was asked for a version for early testing.
> >
> >Applies on top of patch: "virtio_net: fix use after free"
> >that I recently sent.
> >
> >Changes from v3:
> > clean up code, address issues raised by Jason
> >Changes from v1:
> > address comments by Jason Wang, use delayed cb everywhere
> > rebased Jason's patch on top of mine and include it (with some tweaks)
> >
> >Jason Wang (1):
> > virtio-net: optimize free_old_xmit_skbs stats
> >
> >Michael S. Tsirkin (2):
> > virtio_net: enable tx interrupt
> > virtio_net: bql
> >
> > drivers/net/virtio_net.c | 144 +++++++++++++++++++++++++++++++++--------------
> > 1 file changed, 101 insertions(+), 43 deletions(-)
> >
>
> I've run a full tests on this series and see huge regression when zerocopy
> is disabled. Looks like the reason is zerocopy could coalescing tx
> completion which greatly reduce the number of tx interrupts.
I think you refer to this code:
/*
* Trigger polling thread if guest stopped submitting new
* buffers:
* in this case, the refcount after decrement will eventually
* reach 1.
* We also trigger polling periodically after each 16 packets
* (the value 16 here is more or less arbitrary, it's tuned to
* trigger
* less than 10% of times).
*/
if (cnt <= 1 || !(cnt % 16))
vhost_poll_queue(&vq->poll);
?
This seems unrelated to interrupt coalescing.
We can easily enable something similar for all tx
packets, without need for guest configuration.
If it's not clear how to do this, let me know, I'll try to put out a
patch like this in a couple of days.
> I will post RFC V4 shortly with interrupt coalescing support. In this
> version I remove the tx packet cleanup in ndo_start_xmit() since it may
> reduce the effects of interrupt coalescing.
Maybe split this in a separate patch?
^ permalink raw reply
* Re: Re: [bisected] xfrm: TCP connection initiating PMTU discovery stalls on v3.12+
From: Thomas Jarosch @ 2014-12-01 11:20 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, edumazet, Steffen Klassert
In-Reply-To: <20141201102522.GA16579@gondor.apana.org.au>
On Monday, 1. December 2014 18:25:22 Herbert Xu wrote:
> Thomas Jarosch <thomas.jarosch@intra2net.com> wrote:
> > When I revert it, even kernel v3.18-rc6 starts working.
> > But I doubt this is the root problem, may be just hiding another issue.
>
> Can you do a tcpdump with this patch reverted? I would like to
> see the size of the packets that are sent out vs. the ICMP message
> that came back.
>
> My guess is that the IPsec GSO path is buggy since as you rightly
> pointed out it couldn't have been heavily tested prior to this
> patch.
>
> Though I am surprised that it only breaks when you have a PMTU event
> so it might be something else after all.
I've sent you two pcap files off list. One with the reverted patch
and one I created on Saturday showing the stalling TCP connection.
(though the mail currently seems greylisted by the receiving mail server)
One thing I can say from looking at the tcpdump with the reverted patch
is that the ESP packet size drops from 1510 to 1478. The announced MTU
of the next hop in the ICMP message is 1464. It also contains seven
duplicated ACK packets later on.
Without the reverted patch, it sends four ESP packets:
1. 1498 bytes
2. 1498 bytes
3. 1498 bytes
4. 234 bytes
That triggers the ICMP "fragmentation needed" message. I can only spot one
ESP packet of size 1466 afterwards that's sent from time to time. Only one
duplicated ACK packet can be seen. The other packets are just not resent.
May be it's stuck in some in-kernel queue because it's too big to send
and stays there until it expires?
Today I also tried changing the NIC driver on the virtual machine
from virtio_net to e1000. Luckily still the same behavior,
so it's probably not related to the virtio_net driver.
Cheers,
Thomas
^ permalink raw reply
* Re: [PATCH v7 27/46] virtio_net: enable v1.0 support
From: Cornelia Huck @ 2014-12-01 11:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-28-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:11:30 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Now that we have completed 1.0 support, enable it in our driver.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a0e64cf..c6a72d3 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2003,6 +2003,7 @@ static unsigned int features[] = {
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> VIRTIO_NET_F_CTRL_MAC_ADDR,
> VIRTIO_F_ANY_LAYOUT,
> + VIRTIO_F_VERSION_1,
> };
>
> static struct virtio_driver virtio_net_driver = {
Shouldn't you move this after the patch disabling mac address writing?
^ permalink raw reply
* Re: [PATCH v7 34/46] virtio_net: disable mac write for virtio 1.0
From: Cornelia Huck @ 2014-12-01 11:41 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-35-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:12:04 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> The spec states that mac in config space is only driver-writable in the
> legacy case. Fence writing it in virtnet_set_mac_address() in the
> virtio 1.0 case.
>
> Suggested-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v7 27/46] virtio_net: enable v1.0 support
From: Michael S. Tsirkin @ 2014-12-01 11:47 UTC (permalink / raw)
To: Cornelia Huck
Cc: thuth, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <20141201124008.3d8ca343.cornelia.huck@de.ibm.com>
On Mon, Dec 01, 2014 at 12:40:08PM +0100, Cornelia Huck wrote:
> On Sun, 30 Nov 2014 17:11:30 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > Now that we have completed 1.0 support, enable it in our driver.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/net/virtio_net.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index a0e64cf..c6a72d3 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2003,6 +2003,7 @@ static unsigned int features[] = {
> > VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> > VIRTIO_NET_F_CTRL_MAC_ADDR,
> > VIRTIO_F_ANY_LAYOUT,
> > + VIRTIO_F_VERSION_1,
> > };
> >
> > static struct virtio_driver virtio_net_driver = {
>
> Shouldn't you move this after the patch disabling mac address writing?
Probably a good idea.
^ permalink raw reply
* Re: [PATCH v7 29/46] vhost: add memory access wrappers
From: Cornelia Huck @ 2014-12-01 12:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-30-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:11:39 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Add guest memory access wrappers to handle virtio endianness
> conversions.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/vhost.h | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* Re: [PATCH 1/3] net-PPP: Deletion of unnecessary checks before the function call "kfree"
From: Sergei Shtylyov @ 2014-12-01 12:19 UTC (permalink / raw)
To: SF Markus Elfring, Paul Mackerras, linux-ppp, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <547B496E.604@users.sourceforge.net>
Hello.
On 11/30/2014 7:44 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 30 Nov 2014 17:02:07 +0100
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> This issue was detected by using the Coccinelle software.
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/net/ppp/ppp_mppe.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
> diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
> index 911b216..7e44212 100644
> --- a/drivers/net/ppp/ppp_mppe.c
> +++ b/drivers/net/ppp/ppp_mppe.c
> @@ -238,8 +238,7 @@ static void *mppe_alloc(unsigned char *options, int optlen)
> return (void *)state;
>
> out_free:
> - if (state->sha1_digest)
> - kfree(state->sha1_digest);
> + kfree(state->sha1_digest);
Please keep this line aligned to the others.
> if (state->sha1)
> crypto_free_hash(state->sha1);
> if (state->arc4)
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH v7 30/46] vhost/net: force len for TX to host endian
From: Cornelia Huck @ 2014-12-01 12:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-31-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:11:44 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> vhost/net keeps a copy of some used ring but (ab)uses length
> field for internal house-keeping. This works because
> for tx used length is always 0.
> Suppress sparse errors: we use native endian-ness internally but never
> expose it to guest.
I admit that I find this patch description hard to read :)
"vhost/net keeps a copy of the used ring in host memory but (ab)uses
the length field for internal house-keeping. This works because the
length in the used ring for tx is always 0. In order to suppress sparse
errors, we need to force native endianness."
?
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/net.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 8dae2f7..dce5c58 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -48,15 +48,15 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
> * status internally; used for zerocopy tx only.
> */
> /* Lower device DMA failed */
> -#define VHOST_DMA_FAILED_LEN 3
> +#define VHOST_DMA_FAILED_LEN ((__force __virtio32)3)
> /* Lower device DMA done */
> -#define VHOST_DMA_DONE_LEN 2
> +#define VHOST_DMA_DONE_LEN ((__force __virtio32)2)
> /* Lower device DMA in progress */
> -#define VHOST_DMA_IN_PROGRESS 1
> +#define VHOST_DMA_IN_PROGRESS ((__force __virtio32)1)
> /* Buffer unused */
> -#define VHOST_DMA_CLEAR_LEN 0
> +#define VHOST_DMA_CLEAR_LEN ((__force __virtio32)0)
>
> -#define VHOST_DMA_IS_DONE(len) ((len) >= VHOST_DMA_DONE_LEN)
> +#define VHOST_DMA_IS_DONE(len) ((__force u32)(len) >= (__force u32)VHOST_DMA_DONE_LEN)
>
> enum {
> VHOST_NET_FEATURES = VHOST_FEATURES |
^ permalink raw reply
* Re: [PATCH v7 30/46] vhost/net: force len for TX to host endian
From: Michael S. Tsirkin @ 2014-12-01 12:33 UTC (permalink / raw)
To: Cornelia Huck
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <20141201132051.4ab028d9.cornelia.huck@de.ibm.com>
On Mon, Dec 01, 2014 at 01:20:51PM +0100, Cornelia Huck wrote:
> On Sun, 30 Nov 2014 17:11:44 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > vhost/net keeps a copy of some used ring but (ab)uses length
> > field for internal house-keeping. This works because
> > for tx used length is always 0.
> > Suppress sparse errors: we use native endian-ness internally but never
> > expose it to guest.
>
> I admit that I find this patch description hard to read :)
>
>
> "vhost/net keeps a copy of the used ring in host memory but (ab)uses
> the length field for internal house-keeping. This works because the
> length in the used ring for tx is always 0. In order to suppress sparse
> errors, we need to force native endianness."
>
> ?
Yes. Add to this "These values are never exposed to guest."
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Reviewed-by: Jason Wang <jasowang@redhat.com>
> > ---
> > drivers/vhost/net.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 8dae2f7..dce5c58 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -48,15 +48,15 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
> > * status internally; used for zerocopy tx only.
> > */
> > /* Lower device DMA failed */
> > -#define VHOST_DMA_FAILED_LEN 3
> > +#define VHOST_DMA_FAILED_LEN ((__force __virtio32)3)
> > /* Lower device DMA done */
> > -#define VHOST_DMA_DONE_LEN 2
> > +#define VHOST_DMA_DONE_LEN ((__force __virtio32)2)
> > /* Lower device DMA in progress */
> > -#define VHOST_DMA_IN_PROGRESS 1
> > +#define VHOST_DMA_IN_PROGRESS ((__force __virtio32)1)
> > /* Buffer unused */
> > -#define VHOST_DMA_CLEAR_LEN 0
> > +#define VHOST_DMA_CLEAR_LEN ((__force __virtio32)0)
> >
> > -#define VHOST_DMA_IS_DONE(len) ((len) >= VHOST_DMA_DONE_LEN)
> > +#define VHOST_DMA_IS_DONE(len) ((__force u32)(len) >= (__force u32)VHOST_DMA_DONE_LEN)
> >
> > enum {
> > VHOST_NET_FEATURES = VHOST_FEATURES |
^ permalink raw reply
* Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
From: Cornelia Huck @ 2014-12-01 12:33 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-32-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:11:49 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
> 1 file changed, 56 insertions(+), 37 deletions(-)
>
> @@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
> {
> struct vring_desc desc;
> unsigned int i = 0, count, found = 0;
> + u32 len = vhost32_to_cpu(vq, indirect->len);
> int ret;
>
> /* Sanity check */
> - if (unlikely(indirect->len % sizeof desc)) {
> + if (unlikely(len % sizeof desc)) {
> vq_err(vq, "Invalid length in indirect descriptor: "
> "len 0x%llx not multiple of 0x%zx\n",
> - (unsigned long long)indirect->len,
> + (unsigned long long)vhost32_to_cpu(vq, indirect->len),
Can't you use len here?
> sizeof desc);
> return -EINVAL;
> }
>
> - ret = translate_desc(vq, indirect->addr, indirect->len, vq->indirect,
> + ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len, vq->indirect,
> UIO_MAXIOV);
> if (unlikely(ret < 0)) {
> vq_err(vq, "Translation failure %d in indirect.\n", ret);
> @@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
>
> /* Make sure buffer is written before we update index. */
> smp_wmb();
> - if (put_user(vq->last_used_idx, &vq->used->idx)) {
> + if (__put_user(cpu_to_vhost16(vq, vq->last_used_idx), &vq->used->idx)) {
Why s/put_user/__put_user/ - I don't see how endianness conversions
should have an influence there?
> vq_err(vq, "Failed to increment used idx");
> return -EFAULT;
> }
> @@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> if (unlikely(!v))
> return true;
>
> - if (get_user(event, vhost_used_event(vq))) {
> + if (__get_user(event, vhost_used_event(vq))) {
Dito: why the change?
> vq_err(vq, "Failed to get used event idx");
> return true;
> }
> - return vring_need_event(event, new, old);
> + return vring_need_event(vhost16_to_cpu(vq, event), new, old);
> }
>
> /* This actually signals the guest, using eventfd. */
^ permalink raw reply
* Re: [PATCH v7 32/46] vhost/net: virtio 1.0 byte swap
From: Cornelia Huck @ 2014-12-01 12:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-33-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:11:54 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v7 33/46] vhost/net: larger header for virtio 1.0
From: Cornelia Huck @ 2014-12-01 12:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-34-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:11:59 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/net.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
From: Michael S. Tsirkin @ 2014-12-01 12:37 UTC (permalink / raw)
To: Cornelia Huck
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <20141201133353.0bbaa2e1.cornelia.huck@de.ibm.com>
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
> On Sun, 30 Nov 2014 17:11:49 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
> > 1 file changed, 56 insertions(+), 37 deletions(-)
> >
>
> > @@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
> > {
> > struct vring_desc desc;
> > unsigned int i = 0, count, found = 0;
> > + u32 len = vhost32_to_cpu(vq, indirect->len);
> > int ret;
> >
> > /* Sanity check */
> > - if (unlikely(indirect->len % sizeof desc)) {
> > + if (unlikely(len % sizeof desc)) {
> > vq_err(vq, "Invalid length in indirect descriptor: "
> > "len 0x%llx not multiple of 0x%zx\n",
> > - (unsigned long long)indirect->len,
> > + (unsigned long long)vhost32_to_cpu(vq, indirect->len),
>
> Can't you use len here?
Not if I want error message to be readable.
> > sizeof desc);
> > return -EINVAL;
> > }
> >
> > - ret = translate_desc(vq, indirect->addr, indirect->len, vq->indirect,
> > + ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len, vq->indirect,
> > UIO_MAXIOV);
> > if (unlikely(ret < 0)) {
> > vq_err(vq, "Translation failure %d in indirect.\n", ret);
>
>
> > @@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
> >
> > /* Make sure buffer is written before we update index. */
> > smp_wmb();
> > - if (put_user(vq->last_used_idx, &vq->used->idx)) {
> > + if (__put_user(cpu_to_vhost16(vq, vq->last_used_idx), &vq->used->idx)) {
>
> Why s/put_user/__put_user/ - I don't see how endianness conversions
> should have an influence there?
We should generally to __ variants since addresses are pre-validated.
But I agree - should be a separate patch.
>
> > vq_err(vq, "Failed to increment used idx");
> > return -EFAULT;
> > }
>
> > @@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> > if (unlikely(!v))
> > return true;
> >
> > - if (get_user(event, vhost_used_event(vq))) {
> > + if (__get_user(event, vhost_used_event(vq))) {
>
> Dito: why the change?
Same. Will split this out, it's unrelated to virtio 1.0.
> > vq_err(vq, "Failed to get used event idx");
> > return true;
> > }
> > - return vring_need_event(event, new, old);
> > + return vring_need_event(vhost16_to_cpu(vq, event), new, old);
> > }
> >
> > /* This actually signals the guest, using eventfd. */
^ permalink raw reply
* Re: [PATCH v7 36/46] vhost/net: suppress compiler warning
From: Cornelia Huck @ 2014-12-01 12:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <1417359787-10138-37-git-send-email-mst@redhat.com>
On Sun, 30 Nov 2014 17:12:13 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> len is always initialized since function is called with size > 0.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 984242e..54ffbb0 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -501,7 +501,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
> int headcount = 0;
> unsigned d;
> int r, nlogs = 0;
> - u32 len;
> + u32 uninitialized_var(len);
>
> while (datalen > 0 && headcount < quota) {
> if (unlikely(seg >= UIO_MAXIOV)) {
Want to merge this with the patch introducing the variable and add a
comment there?
^ permalink raw reply
* Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
From: Cornelia Huck @ 2014-12-01 12:42 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <20141201123701.GB17958@redhat.com>
On Mon, 1 Dec 2014 14:37:01 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
> > On Sun, 30 Nov 2014 17:11:49 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
> > > 1 file changed, 56 insertions(+), 37 deletions(-)
> > >
> >
> > > @@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
> > > {
> > > struct vring_desc desc;
> > > unsigned int i = 0, count, found = 0;
> > > + u32 len = vhost32_to_cpu(vq, indirect->len);
> > > int ret;
> > >
> > > /* Sanity check */
> > > - if (unlikely(indirect->len % sizeof desc)) {
> > > + if (unlikely(len % sizeof desc)) {
> > > vq_err(vq, "Invalid length in indirect descriptor: "
> > > "len 0x%llx not multiple of 0x%zx\n",
> > > - (unsigned long long)indirect->len,
> > > + (unsigned long long)vhost32_to_cpu(vq, indirect->len),
> >
> > Can't you use len here?
>
> Not if I want error message to be readable.
Huh? Both have the same value.
^ permalink raw reply
* HELLO
From: matildawoart @ 2014-12-01 12:47 UTC (permalink / raw)
My Dearest, how are you today? I hope you are fine, I am
Barrister Matilda woart from woart & Associate Chambers. I have something very
important
to discuss with you and you can contact me via my private
email:
Matildawoart@gmail.com
Best Regards,
Honorable Matilda woart solicitor-at-law (CDF, LLM. BL)
Office: 20 Rue du Chenin de Fer
Matilda woart
Liberia.
^ permalink raw reply
* Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
From: Michael S. Tsirkin @ 2014-12-01 12:49 UTC (permalink / raw)
To: Cornelia Huck
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <20141201134247.55e68161.cornelia.huck@de.ibm.com>
On Mon, Dec 01, 2014 at 01:42:47PM +0100, Cornelia Huck wrote:
> On Mon, 1 Dec 2014 14:37:01 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
> > > On Sun, 30 Nov 2014 17:11:49 +0200
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > > drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
> > > > 1 file changed, 56 insertions(+), 37 deletions(-)
> > > >
> > >
> > > > @@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
> > > > {
> > > > struct vring_desc desc;
> > > > unsigned int i = 0, count, found = 0;
> > > > + u32 len = vhost32_to_cpu(vq, indirect->len);
> > > > int ret;
> > > >
> > > > /* Sanity check */
> > > > - if (unlikely(indirect->len % sizeof desc)) {
> > > > + if (unlikely(len % sizeof desc)) {
> > > > vq_err(vq, "Invalid length in indirect descriptor: "
> > > > "len 0x%llx not multiple of 0x%zx\n",
> > > > - (unsigned long long)indirect->len,
> > > > + (unsigned long long)vhost32_to_cpu(vq, indirect->len),
> > >
> > > Can't you use len here?
> >
> > Not if I want error message to be readable.
>
> Huh? Both have the same value.
Ah, good point.
^ permalink raw reply
* Re: tun issue after e0b46d0ee9c: tun: Use iovec iterators
From: Marcelo Ricardo Leitner @ 2014-12-01 13:05 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20141130082106.GA9531@gondor.apana.org.au>
On 30-11-2014 06:21, Herbert Xu wrote:
> On Sat, Nov 29, 2014 at 03:08:09AM -0200, Marcelo Ricardo Leitner wrote:
>> On 28-11-2014 21:59, Herbert Xu wrote:
>>> On Fri, Nov 28, 2014 at 05:25:27PM -0200, Marcelo Ricardo Leitner wrote:
>>>>
>>>> I saw there are tun updates on Dave's queue but none seemed to handle this.
>>>>
>>>> I can't use current net-next
>>>> (799d2fff1858004526ad75d66a5dd8a5cce6ad40) on a kvm hypervisor
>>>> because tun got clogged somehow. Bisected down to:
>>>>
>>>> commit e0b46d0ee9c240c7430a47e9b0365674d4a04522
>>>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>>>> Date: Fri Nov 7 21:22:23 2014 +0800
>>>
>>> Does this patch help?
>>
>> Yay, it does! Works for me, thanks Herbert.
>> I didn't test performance, but dhcp could get through.
>>
>> Are you sure about the Fixes tag? Because bisect really pointed to e0b46d0ee9c.
>
> Well according to your report you were having problems with
> tun_get_user. The bug I introduced was in tun_put_user and has
> already been fixed by Jason Wang.
Ahh yes, ok. Thanks
Cheers,
Marcelo
^ permalink raw reply
* Re: [bisected] xfrm: TCP connection initiating PMTU discovery stalls on v3.12+
From: Wolfgang Walter @ 2014-12-01 13:17 UTC (permalink / raw)
To: Thomas Jarosch; +Cc: netdev, Eric Dumazet
In-Reply-To: <1709726.jUgUSQI9sl@pikkukde.a.i2n>
Am Samstag, 29. November 2014, 12:44:07 schrieb Thomas Jarosch:
> Hello,
>
> we're in the process of updating production level machines
> from kernel 3.4.101 to kernel 3.14.25. On one mail server
> we noticed that emails destined for an IPSec tunnel sometimes
> get stuck in the mail queue with TCP timeouts.
>
> To make a long story short: When the VPN connection is initially
> set up or re-newed, the path MTU for the xfrm tunnel is undetermined.
>
> As soon as a TCP client starts to send large packets,
> it triggers path MTU detection. Some middlebox on the
> way to the final server has a lower MTU and sends back
> an "ICMP fragmentation needed" packet as normal.
>
> With the old kernel, the packet size for the TCP connection inside
> the xfrm tunnel gets adjusted and all is fine. With kernel v3.12+,
> the connection stalls completely. Same thing with kernel v3.18-rc6.
We see something similar with real nic (RTL8139). In our case only the first
tcp-connection which triggers PMTU stalls. Later tcp-connections then work
fine.
I will revert that patch and see if that fixes the problem.
>
> We wrote a small tool to mimic postfix's TCP behavior (see attached file).
> In the end it's a normal TCP client sending large packets.
> The server side is just "socat - tcp4-listen:667".
>
> If you run "socket_client" a second time, the path MTU
> for the xfrm tunnel is already known and packets flow normal, too.
>
>
> The "evil" commit in question is this one:
> ---------------------------------------------------------------------
> commit 8f26fb1c1ed81c33f5d87c5936f4d9d1b4118918
> Author: Eric Dumazet <edumazet@google.com>
> Date: Tue Oct 15 12:24:54 2013 -0700
>
> tcp: remove the sk_can_gso() check from tcp_set_skb_tso_segs()
>
> sk_can_gso() should only be used as a hint in tcp_sendmsg() to build GSO
> packets in the first place. (As a performance hint)
>
> Once we have GSO packets in write queue, we can not decide they are no
> longer GSO only because flow now uses a route which doesn't handle
> TSO/GSO.
>
> Core networking stack handles the case very well for us, all we need
> is keeping track of packet counts in MSS terms, regardless of
> segmentation done later (in GSO or hardware)
>
> Right now, if tcp_fragment() splits a GSO packet in two parts,
> @left and @right, and route changed through a non GSO device,
> both @left and @right have pcount set to 1, which is wrong,
> and leads to incorrect packet_count tracking.
>
> This problem was added in commit d5ac99a648 ("[TCP]: skb pcount with MTU
> discovery")
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Reported-by: Maciej Żenczykowski <maze@google.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 8fad1c1..d46f214 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -989,8 +989,7 @@ static void tcp_set_skb_tso_segs(const struct sock *sk,
> struct sk_buff *skb, /* Make sure we own this skb before messing
> gso_size/gso_segs */ WARN_ON_ONCE(skb_cloned(skb));
>
> - if (skb->len <= mss_now || !sk_can_gso(sk) ||
> - skb->ip_summed == CHECKSUM_NONE) {
> + if (skb->len <= mss_now || skb->ip_summed == CHECKSUM_NONE) {
> /* Avoid the costly divide in the normal
> * non-TSO case.
> */
> ---------------------------------------------------------------------
>
> When I revert it, even kernel v3.18-rc6 starts working.
> But I doubt this is the root problem, may be just hiding another issue.
>
> --- Sample output of socket_client using vanilla v3.12 kernel ---
> [1417258063 SEND result: 4096, strerror: Success]
> tcp max seg: res: 0, max_seg: 1370
> [1417258063 SEND result: 4096, strerror: Success]
> tcp max seg: res: 0, max_seg: 1370
> [1417258063 SEND result: 4096, strerror: Success]
> tcp max seg: res: 0, max_seg: 1370
> [1417258063 SEND result: 4096, strerror: Success]
> tcp max seg: res: 0, max_seg: 1370
> [1417258063 SEND result: 4096, strerror: Success]
> tcp max seg: res: 0, max_seg: 1338
> [1417258063 SEND result: 4096, strerror: Success]
> tcp max seg: res: 0, max_seg: 1338
> *STUCK*
> --------------------------------------------------------
>
> The "machine" is running on KVM and using "virtio_net" as NIC driver.
> I've played with the ethtool offload settings:
>
> *** eth1 defaults ***
> Offload parameters for eth1:
> rx-checksumming: on
> tx-checksumming: on
> scatter-gather: on
> tcp-segmentation-offload: on
> udp-fragmentation-offload: on
> generic-segmentation-offload: on
> generic-receive-offload: on
> large-receive-offload: off
>
> *** eth1 working (no stalls) using vanilla kernel ***
> Offload parameters for eth1:
> rx-checksumming: on
> tx-checksumming: off <-- the magic switch
> scatter-gather: off
> tcp-segmentation-offload: off
> udp-fragmentation-offload: off
> generic-segmentation-offload: off
> generic-receive-offload: off
> large-receive-offload: off
>
> When I turn "tx-checksumming" back on, it fails again.
> Though that is probably also just a side effect.
>
> I can provide tcpdumps if needed but they are no real help
> since you can just see the kernel stops sending TCP packets.
> (and the outgoing TCP packets are encrypted in ESP packets)
>
>
> Any vague idea what might be the root cause?
>
> I also tried reverting commit 4d53eff48b5f03ce67f4f301d6acca1d2145cb7a
> ("xfrm: Don't queue retransmitted packets if the original is still on the
> host") but that didn't change the situation. In fact it wasn't even
> triggered.
>
> Please CC: comments. Thanks.
>
> Best regards,
> Thomas
Regards,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
^ permalink raw reply
* Re: [Xen-devel] [PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary
From: David Vrabel @ 2014-12-01 13:27 UTC (permalink / raw)
To: David Miller, seth.forshee
Cc: zoltan.kiss, eric.dumazet, netdev, linux-kernel, stefan.bader,
david.vrabel, xen-devel, boris.ostrovsky
In-Reply-To: <20141126.122812.223757363894961994.davem@davemloft.net>
On 26/11/14 17:28, David Miller wrote:
> From: Seth Forshee <seth.forshee@canonical.com>
> Date: Tue, 25 Nov 2014 20:28:24 -0600
>
>> These BUGs can be erroneously triggered by frags which refer to
>> tail pages within a compound page. The data in these pages may
>> overrun the hardware page while still being contained within the
>> compound page, but since compound_order() evaluates to 0 for tail
>> pages the assertion fails. The code already iterates through
>> subsequent pages correctly in this scenario, so the BUGs are
>> unnecessary and can be removed.
>>
>> Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
>> Cc: <stable@vger.kernel.org> # 3.7+
>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>
> Can I get some Xen developer reviews?
Sorry for the delay, I've been on holiday.
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Thanks.
David
^ permalink raw reply
* Re: [Xen-devel] [PATCH] xen-netfront: Fix handling packets on compound pages with skb_linearize
From: David Vrabel @ 2014-12-01 13:36 UTC (permalink / raw)
To: Stefan Bader, Zoltan Kiss, Konrad Rzeszutek Wilk, Boris Ostrovsky,
David Vrabel
Cc: Wei Liu, Ian Campbell, netdev, linux-kernel, Paul Durrant,
xen-devel
In-Reply-To: <547C2CFC.7060908@canonical.com>
On 01/12/14 08:55, Stefan Bader wrote:
> On 11.08.2014 19:32, Zoltan Kiss wrote:
>> There is a long known problem with the netfront/netback interface: if the guest
>> tries to send a packet which constitues more than MAX_SKB_FRAGS + 1 ring slots,
>> it gets dropped. The reason is that netback maps these slots to a frag in the
>> frags array, which is limited by size. Having so many slots can occur since
>> compound pages were introduced, as the ring protocol slice them up into
>> individual (non-compound) page aligned slots. The theoretical worst case
>> scenario looks like this (note, skbs are limited to 64 Kb here):
>> linear buffer: at most PAGE_SIZE - 17 * 2 bytes, overlapping page boundary,
>> using 2 slots
>> first 15 frags: 1 + PAGE_SIZE + 1 bytes long, first and last bytes are at the
>> end and the beginning of a page, therefore they use 3 * 15 = 45 slots
>> last 2 frags: 1 + 1 bytes, overlapping page boundary, 2 * 2 = 4 slots
>> Although I don't think this 51 slots skb can really happen, we need a solution
>> which can deal with every scenario. In real life there is only a few slots
>> overdue, but usually it causes the TCP stream to be blocked, as the retry will
>> most likely have the same buffer layout.
>> This patch solves this problem by linearizing the packet. This is not the
>> fastest way, and it can fail much easier as it tries to allocate a big linear
>> area for the whole packet, but probably easier by an order of magnitude than
>> anything else. Probably this code path is not touched very frequently anyway.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Ian Campbell <Ian.Campbell@citrix.com>
>> Cc: Paul Durrant <paul.durrant@citrix.com>
>> Cc: netdev@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: xen-devel@lists.xenproject.org
>
> This does not seem to be marked explicitly as stable. Has someone already asked
> David Miller to put it on his stable queue? IMO it qualifies quite well and the
> actual change should be simple to pick/backport.
I think it's a candidate, yes.
Can you expand on the user visible impact of the bug this patch fixes?
I think it results in certain types of traffic not working (because the
domU always generates skb's with the problematic frag layout), but I
can't remember the details.
David
^ permalink raw reply
* Re: [PATCH v7 36/46] vhost/net: suppress compiler warning
From: Michael S. Tsirkin @ 2014-12-01 13:48 UTC (permalink / raw)
To: Cornelia Huck
Cc: thuth, kvm, rusty, netdev, linux-kernel, virtualization, dahi,
pbonzini, David Miller
In-Reply-To: <20141201133740.56d7d6d8.cornelia.huck@de.ibm.com>
On Mon, Dec 01, 2014 at 01:37:40PM +0100, Cornelia Huck wrote:
> On Sun, 30 Nov 2014 17:12:13 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > len is always initialized since function is called with size > 0.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/vhost/net.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 984242e..54ffbb0 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -501,7 +501,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
> > int headcount = 0;
> > unsigned d;
> > int r, nlogs = 0;
> > - u32 len;
> > + u32 uninitialized_var(len);
> >
> > while (datalen > 0 && headcount < quota) {
> > if (unlikely(seg >= UIO_MAXIOV)) {
>
> Want to merge this with the patch introducing the variable and add a
> comment there?
Not really. Warnings in bisect are fine I think.
--
MST
^ permalink raw reply
* Re: [PATCH] mips: bpf: Fix broken BPF_MOD
From: Sergei Shtylyov @ 2014-12-01 13:50 UTC (permalink / raw)
To: Denis Kirjanov, netdev; +Cc: markos.chandras
In-Reply-To: <1417427822-12729-1-git-send-email-kda@linux-powerpc.org>
Hello.
On 12/1/2014 12:57 PM, Denis Kirjanov wrote:
You should CC the 'linux-mips' ML.
> Remove optimize_div() from BPF_MOD | BPF_K case
> since we don't know the dividend and fix the
> emit_mod() by reading the mod operation result from HI register
Isn't this 2 unrelated fixes? They should be in 2 patches, not a single
one in that case.
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
> arch/mips/net/bpf_jit.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Thomas Graf @ 2014-12-01 13:52 UTC (permalink / raw)
To: Du, Fan
Cc: 'Jason Wang', netdev@vger.kernel.org, davem@davemloft.net,
fw@strlen.de, dev, mst, jesse, pshelar
In-Reply-To: <5A90DA2E42F8AE43BC4A093BF0678848DED92B@SHSMSX104.ccr.corp.intel.com>
On 11/30/14 at 10:08am, Du, Fan wrote:
> >-----Original Message-----
> >From: Jason Wang [mailto:jasowang@redhat.com]
> >Sent: Friday, November 28, 2014 3:02 PM
> >To: Du, Fan
> >Cc: netdev@vger.kernel.org; davem@davemloft.net; fw@strlen.de; Du, Fan
> >Subject: Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
> >On Fri, Nov 28, 2014 at 2:33 PM, Fan Du <fan.du@intel.com> wrote:
> >> Test scenario: two KVM guests sitting in different hosts communicate
> >> to each other with a vxlan tunnel.
> >>
> >> All interface MTU is default 1500 Bytes, from guest point of view, its
> >> skb gso_size could be as bigger as 1448Bytes, however after guest skb
> >> goes through vxlan encapuslation, individual segments length of a gso
> >> packet could exceed physical NIC MTU 1500, which will be lost at
> >> recevier side.
> >>
> >> So it's possible in virtualized environment, locally created skb len
> >> after encapslation could be bigger than underlayer MTU. In such case,
> >> it's reasonable to do GSO first, then fragment any packet bigger than
> >> MTU as possible.
> >>
> >> +---------------+ TX RX +---------------+
> >> | KVM Guest | -> ... -> | KVM Guest |
> >> +-+-----------+-+ +-+-----------+-+
> >> |Qemu/VirtIO| |Qemu/VirtIO|
> >> +-----------+ +-----------+
> >> | |
> >> v tap0 tap0 v
> >> +-----------+ +-----------+
> >> | ovs bridge| | ovs bridge|
> >> +-----------+ +-----------+
> >> | vxlan vxlan |
> >> v v
> >> +-----------+ +-----------+
> >> | NIC | <------> | NIC |
> >> +-----------+ +-----------+
> >>
> >> Steps to reproduce:
> >> 1. Using kernel builtin openvswitch module to setup ovs bridge.
> >> 2. Runing iperf without -M, communication will stuck.
> >
> >Is this issue specific to ovs or ipv4? Path MTU discovery should help in this case I
> >believe.
>
> Problem here is host stack push local over-sized gso skb down to NIC, and perform GSO there
> without any further ip segmentation.
>
> Reasonable behavior is do gso first at ip level, if gso-ed skb is bigger than MTU && df is set,
> Then push ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED message back to sender to adjust mtu.
Aside from this. I think Virtio should provide a MTU hint to the guest
to adjust MTU in the vNIC to account for both overhead or support for
jumbo frames in the underlay transparently without relying on PMTU or
MSS hints. I remember we talked about this a while ago with at least
Michael but haven't done actual code work on it yet.
> For PMTU to work, that's another issue I will try to address later on.
PMTU discovery was explicitly removed from the OVS datapath. Maybe
Pravin or Jesse can provide some background on that
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox