* Re: DMA allocations from CMA and fatal_signal_pending check
From: Florian Fainelli @ 2014-11-03 18:51 UTC (permalink / raw)
To: Michal Nazarewicz, Joonsoo Kim
Cc: linux-arm-kernel, Brian Norris, Gregory Fong, linux-kernel,
linux-mm, lauraa, gioh.kim, aneesh.kumar, m.szyprowski, akpm,
netdev@vger.kernel.org
In-Reply-To: <xa1tlhnsw7v8.fsf@mina86.com>
On 11/03/2014 08:45 AM, Michal Nazarewicz wrote:
> On Fri, Oct 31 2014, Florian Fainelli wrote:
>> I agree that the CMA allocation should not be allowed to succeed, but
>> the dma_alloc_coherent() allocation should succeed. If we look at the
>> sysport driver, there are kmalloc() calls to initialize private
>> structures, those will succeed (except under high memory pressure), so
>> by the same token, a driver expects DMA allocations to succeed (unless
>> we are under high memory pressure)
>>
>> What are we trying to solve exactly with the fatal_signal_pending()
>> check here? Are we just optimizing for the case where a process has
>> allocated from a CMA region to allow this region to be returned to the
>> pool of free pages when it gets killed? Could there be another mechanism
>> used to reclaim those pages if we know the process is getting killed
>> anyway?
>
> We're guarding against situations where process may hang around
> arbitrarily long time after receiving SIGKILL. If user does “kill -9
> $pid” the usual expectation is that the $pid process will die within
> seconds and anything longer is perceived by user as a bug.
>
> What problem are *you* trying to solve? If user sent SIGKILL to
> a process that imitated device initialisation, what is the point of
> continuing initialising the device? Just recover and return -EINTR.
I have two problems with the current approach:
- behavior of a dma_alloc_coherent() call is not consistent between a
CONFIG_CMA=y vs. CONFIG_CMA=n build, which is probably fine as long as
we document that properly
- there is currently no way for a caller of dma_alloc_coherent to tell
whether the allocation failed because it was interrupted by a signal, a
genuine OOM or something else, this is largely made worse by problem 1
>
>> Well, not really. This driver is not an isolated case, there are tons of
>> other networking drivers that do exactly the same thing, and we do
>> expect these dma_alloc_* calls to succeed.
>
> Again, why do you expect them to succeed? The code must handle failures
> correctly anyway so why do you wish to ignore fatal signal?
I guess expecting them to succeed is probably not good, but at we should
at least be able to report an accurate error code to the caller and down
to user-space.
Thanks
--
Florian
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] net: eth: realtek: atp: checkpatch errors and warnings corrected
From: Joe Perches @ 2014-11-03 18:51 UTC (permalink / raw)
To: Roberto Medina; +Cc: netdev, linux-kernel
In-Reply-To: <1415039073-20164-1-git-send-email-robertoxmed@gmail.com>
On Mon, 2014-11-03 at 19:24 +0100, Roberto Medina wrote:
> From: Roberto Medina <robertoxmed@gmail.com>
>
> Several warnings and errors corrected using checkpatch.
> Some warnings like "line over 80" are still present.
>
> Before the patch:
> total: 16 errors, 155 warnings, 38 checks, 883 lines checked
>
> With the patch:
> total: 0 errors, 64 warnings, 24 checks, 905 lines checked
>
> Compile tested.
Some ancient drivers could be regarded as neolithic
curiosities that never need updating. This may be one.
But if you really want to change it, could you please
make sure that objdiff shows no changes?
^ permalink raw reply
* Re: [PATCH net-next 3/7] gue: Add infrastructure for flags and options
From: Tom Herbert @ 2014-11-03 18:39 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <20141103.121856.1307429117331581815.davem@davemloft.net>
On Mon, Nov 3, 2014 at 9:18 AM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Sat, 1 Nov 2014 15:57:59 -0700
>
>> @@ -20,7 +20,16 @@ static size_t fou_encap_hlen(struct ip_tunnel_encap *e)
>>
>> static size_t gue_encap_hlen(struct ip_tunnel_encap *e)
>> {
>> - return sizeof(struct udphdr) + sizeof(struct guehdr);
>> + size_t len;
>> + bool need_priv = false;
>> +
>> + len = sizeof(struct udphdr) + sizeof(struct guehdr);
>> +
>> + /* Add in lengths flags */
>> +
>> + len += need_priv ? GUE_LEN_PRIV : 0;
>
> Add this need_priv logic in patch #6, not here.
I would rather keep it in this patch. This is adding the common
infrastructure to support private option field, remote checksum
offload is an instance that uses that.
Tom
^ permalink raw reply
* Re: [PATCH 1/7] can: m_can: fix possible sleep in napi poll
From: Oliver Hartkopp @ 2014-11-03 18:37 UTC (permalink / raw)
To: Marc Kleine-Budde, Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <5457B517.8030805@pengutronix.de>
Hello Marc,
you can omit the 'stable' tag as this driver just emerged in 3.18 - which is
currently in 3.18-rc3 state.
If possible I would suggest to push all 7 patches via can/master as it is
pretty bad to say "M_CAN was introduced in 3.18 but M_CAN with CAN FD is
working since 3.19".
This will produce confusion as the M_CAN core has built-in CAN FD support.
Regards,
Oliver
On 11/03/2014 06:02 PM, Marc Kleine-Budde wrote:
> Hey Dong Aisheng,
>
> the state of the patches are:
>
> 1: can/master + stable
> 2: can/mster
> 3: can/mster
> 4: can/master + stable
> 5: waiting for Oliver's opinion due to the user space change
> 6: waiting for your input
> 7: waiting for your input
>
> Marc
>
^ permalink raw reply
* Re: [PATCH] VNIC: Adding support for Cavium ThunderX network controller
From: Robert Richter @ 2014-11-03 18:33 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Sunil Kovvuri, Robert Richter, David S. Miller, Sunil Goutham,
Stefan Assmann, LKML, LAKML, netdev
In-Reply-To: <20141103101651.1d664cc3@urahara>
On 03.11.14 10:16:51, Stephen Hemminger wrote:
> On Fri, 31 Oct 2014 22:44:11 +0530
> Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> > On Fri, Oct 31, 2014 at 8:24 AM, Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > > On Thu, 30 Oct 2014 17:54:34 +0100
> > > Robert Richter <rric@kernel.org> wrote:
> > >
> > >> +#ifdef VNIC_RSS_SUPPORT
> > >> +static int rss_config = RSS_IP_HASH_ENA | RSS_TCP_HASH_ENA | RSS_UDP_HASH_ENA;
> > >> +module_param(rss_config, int, S_IRUGO);
> > >> +MODULE_PARM_DESC(rss_config,
> > >> + "RSS hash config [bits 8:0] (Bit0:L2 extended, 1:IP, 2:TCP, 3:TCP SYN, 4:UDP, 5:L4 extended, 6:ROCE 7:L3 bi-directional, 8:L4 bi-directional)");
> > >> +#endif
> > >
> > > This should managed be via ethtool ETHTOOL_GRXFH rather than a module parameter.
> > Thanks, i will add setting hash options via ETHTOOL_SRXFH as well.
> > The idea here is to have a choice of hash while module load (through
> > module params) and if it needs to be changed runtime then
> > via Ethtool.
> >
> > Sunil.
>
> Network developers do not like vendor unique module parameters.
> Anything device specific doesn't work in a generic distro environment.
Do you accept unique module parameters in parallel to ethtool support
or should this be removed?
-Robert
^ permalink raw reply
* Re: [PATCH 5/7] can: clear ctrlmode when close candev
From: Oliver Hartkopp @ 2014-11-03 18:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Dong Aisheng, Wolfgang Grandegger
Cc: linux-can, wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <5457AD40.7020606@pengutronix.de>
On 11/03/2014 05:28 PM, Marc Kleine-Budde wrote:
> On 10/29/2014 11:45 AM, Dong Aisheng wrote:
>> Currently priv->ctrlmode is not cleared when close_candev, so next time
>> the driver will still use this value to set controller even user
>> does not set any ctrl mode.
>> e.g.
>> Step 1. ip link set can0 up type can0 bitrate 1000000 loopback on
>> Controller will be in loopback mode
>> Step 2. ip link set can0 down
>> Step 3. ip link set can0 up type can0 bitrate 1000000
>> Controller will still be set to loopback mode in driver due to saved
>> priv->ctrlmode.
>>
>> This patch clears priv->ctrlmode when the CAN interface is closed,
>> and set it to correct mode according to next user setting.
>
> Oliver, what do you think of this patch? It will introduce a subtle
> change to the userspace.
Hi Marc,
indeed the current policy is that ifdown/ifup doesn't change any setting.
When we start to clear priv->ctrlmode when setting the interface down the
question arises whether the bitrate(s) has/have to be wiped too.
Setting the interface down/up can be done with ip or ifconfig without changing
the controller settings.
So IMO we can not assume that people always use the 'all-in-one'
ip link set can0 up type can0 bitrate 1000000
command as given in the example above.
You might also do it like this:
ip link set can0 up type can0 bitrate 1000000 loopback on
ip link set can0 down
ip link set can0 type can0 loopback off
ip link set can0 up
Finally I would vote to reject this patch to stay consistent.
When users like to fiddle with options like loopback they need to take care
about their 'expert' settings too.
Regards,
Oliver
>
>> Signed-off-by: Dong Aisheng <b29396@freescale.com>
>> ---
>> drivers/net/can/dev.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>> index 02492d2..1fce485 100644
>> --- a/drivers/net/can/dev.c
>> +++ b/drivers/net/can/dev.c
>> @@ -671,6 +671,7 @@ void close_candev(struct net_device *dev)
>>
>> del_timer_sync(&priv->restart_timer);
>> can_flush_echo_skb(dev);
>> + priv->ctrlmode = 0;
>> }
>> EXPORT_SYMBOL_GPL(close_candev);
>>
>>
>
>
^ permalink raw reply
* [PATCH] net: eth: realtek: atp: checkpatch errors and warnings corrected
From: Roberto Medina @ 2014-11-03 18:24 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Roberto Medina
From: Roberto Medina <robertoxmed@gmail.com>
Several warnings and errors corrected using checkpatch.
Some warnings like "line over 80" are still present.
Before the patch:
total: 16 errors, 155 warnings, 38 checks, 883 lines checked
With the patch:
total: 0 errors, 64 warnings, 24 checks, 905 lines checked
Compile tested.
Signed-off-by: Roberto Medina <robertoxmed@gmail.com>
---
drivers/net/ethernet/realtek/atp.c | 470 +++++++++++++++++++------------------
1 file changed, 246 insertions(+), 224 deletions(-)
diff --git a/drivers/net/ethernet/realtek/atp.c b/drivers/net/ethernet/realtek/atp.c
index d77d60e..8291260 100644
--- a/drivers/net/ethernet/realtek/atp.c
+++ b/drivers/net/ethernet/realtek/atp.c
@@ -1,43 +1,42 @@
/* atp.c: Attached (pocket) ethernet adapter driver for linux. */
-/*
- This is a driver for commonly OEM pocket (parallel port)
- ethernet adapters based on the Realtek RTL8002 and RTL8012 chips.
-
- Written 1993-2000 by Donald Becker.
-
- This software may be used and distributed according to the terms of
- the GNU General Public License (GPL), incorporated herein by reference.
- Drivers based on or derived from this code fall under the GPL and must
- retain the authorship, copyright and license notice. This file is not
- a complete program and may only be used when the entire operating
- system is licensed under the GPL.
-
- Copyright 1993 United States Government as represented by the Director,
- National Security Agency. Copyright 1994-2000 retained by the original
- author, Donald Becker. The timer-based reset code was supplied in 1995
- by Bill Carlson, wwc@super.org.
-
- The author may be reached as becker@scyld.com, or C/O
- Scyld Computing Corporation
- 410 Severn Ave., Suite 210
- Annapolis MD 21403
-
- Support information and updates available at
- http://www.scyld.com/network/atp.html
-
-
- Modular support/softnet added by Alan Cox.
- _bit abuse fixed up by Alan Cox
-
-*/
+/* This is a driver for commonly OEM pocket (parallel port)
+ * ethernet adapters based on the Realtek RTL8002 and RTL8012 chips.
+ *
+ * Written 1993-2000 by Donald Becker.
+ *
+ * This software may be used and distributed according to the terms of
+ * the GNU General Public License (GPL), incorporated herein by reference.
+ * Drivers based on or derived from this code fall under the GPL and must
+ * retain the authorship, copyright and license notice. This file is not
+ * a complete program and may only be used when the entire operating
+ * system is licensed under the GPL.
+ *
+ * Copyright 1993 United States Government as represented by the Director,
+ * National Security Agency. Copyright 1994-2000 retained by the original
+ * author, Donald Becker. The timer-based reset code was supplied in 1995
+ * by Bill Carlson, wwc@super.org.
+ *
+ * The author may be reached as becker@scyld.com, or C/O
+ * Scyld Computing Corporation
+ * 410 Severn Ave., Suite 210
+ * Annapolis MD 21403
+ *
+ * Support information and updates available at
+ * http://www.scyld.com/network/atp.html
+ *
+ * Modular support/softnet added by Alan Cox.
+ * _bit abuse fixed up by Alan Cox
+ *
+ */
static const char version[] =
"atp.c:v1.09=ac 2002/10/01 Donald Becker <becker@scyld.com>\n";
/* The user-configurable values.
- These may be modified when a driver module is loaded.*/
+ * These may be modified when a driver module is loaded.
+ */
-static int debug = 1; /* 1 normal messages, 0 quiet .. 7 verbose. */
+static int debug = 1; /* 1 normal messages, 0 quiet .. 7 verbose. */
#define net_debug debug
/* Maximum events (Rx packets, etc.) to handle at each interrupt. */
@@ -47,80 +46,79 @@ static int max_interrupt_work = 15;
/* The standard set of ISA module parameters. */
static int io[NUM_UNITS];
static int irq[NUM_UNITS];
-static int xcvr[NUM_UNITS]; /* The data transfer mode. */
+static int xcvr[NUM_UNITS]; /* The data transfer mode. */
/* Operational parameters that are set at compile time. */
/* Time in jiffies before concluding the transmitter is hung. */
#define TX_TIMEOUT (400*HZ/1000)
-/*
- This file is a device driver for the RealTek (aka AT-Lan-Tec) pocket
- ethernet adapter. This is a common low-cost OEM pocket ethernet
- adapter, sold under many names.
-
- Sources:
- This driver was written from the packet driver assembly code provided by
- Vincent Bono of AT-Lan-Tec. Ever try to figure out how a complicated
- device works just from the assembly code? It ain't pretty. The following
- description is written based on guesses and writing lots of special-purpose
- code to test my theorized operation.
-
- In 1997 Realtek made available the documentation for the second generation
- RTL8012 chip, which has lead to several driver improvements.
- http://www.realtek.com.tw/
-
- Theory of Operation
-
- The RTL8002 adapter seems to be built around a custom spin of the SEEQ
- controller core. It probably has a 16K or 64K internal packet buffer, of
- which the first 4K is devoted to transmit and the rest to receive.
- The controller maintains the queue of received packet and the packet buffer
- access pointer internally, with only 'reset to beginning' and 'skip to next
- packet' commands visible. The transmit packet queue holds two (or more?)
- packets: both 'retransmit this packet' (due to collision) and 'transmit next
- packet' commands must be started by hand.
-
- The station address is stored in a standard bit-serial EEPROM which must be
- read (ughh) by the device driver. (Provisions have been made for
- substituting a 74S288 PROM, but I haven't gotten reports of any models
- using it.) Unlike built-in devices, a pocket adapter can temporarily lose
- power without indication to the device driver. The major effect is that
- the station address, receive filter (promiscuous, etc.) and transceiver
- must be reset.
-
- The controller itself has 16 registers, some of which use only the lower
- bits. The registers are read and written 4 bits at a time. The four bit
- register address is presented on the data lines along with a few additional
- timing and control bits. The data is then read from status port or written
- to the data port.
-
- Correction: the controller has two banks of 16 registers. The second
- bank contains only the multicast filter table (now used) and the EEPROM
- access registers.
-
- Since the bulk data transfer of the actual packets through the slow
- parallel port dominates the driver's running time, four distinct data
- (non-register) transfer modes are provided by the adapter, two in each
- direction. In the first mode timing for the nibble transfers is
- provided through the data port. In the second mode the same timing is
- provided through the control port. In either case the data is read from
- the status port and written to the data port, just as it is accessing
- registers.
-
- In addition to the basic data transfer methods, several more are modes are
- created by adding some delay by doing multiple reads of the data to allow
- it to stabilize. This delay seems to be needed on most machines.
-
- The data transfer mode is stored in the 'dev->if_port' field. Its default
- value is '4'. It may be overridden at boot-time using the third parameter
- to the "ether=..." initialization.
-
- The header file <atp.h> provides inline functions that encapsulate the
- register and data access methods. These functions are hand-tuned to
- generate reasonable object code. This header file also documents my
- interpretations of the device registers.
-*/
+/* This file is a device driver for the RealTek (aka AT-Lan-Tec) pocket
+ * ethernet adapter. This is a common low-cost OEM pocket ethernet
+ * adapter, sold under many names.
+ *
+ * Sources:
+ * This driver was written from the packet driver assembly code provided by
+ * Vincent Bono of AT-Lan-Tec. Ever try to figure out how a complicated
+ * device works just from the assembly code? It ain't pretty. The following
+ * description is written based on guesses and writing lots of special-purpose
+ * code to test my theorized operation.
+ *
+ * In 1997 Realtek made available the documentation for the second generation
+ * RTL8012 chip, which has lead to several driver improvements.
+ * http://www.realtek.com.tw/
+ *
+ * Theory of Operation
+ *
+ * The RTL8002 adapter seems to be built around a custom spin of the SEEQ
+ * controller core. It probably has a 16K or 64K internal packet buffer, of
+ * which the first 4K is devoted to transmit and the rest to receive.
+ * The controller maintains the queue of received packet and the packet buffer
+ * access pointer internally, with only 'reset to beginning' and 'skip to next
+ * packet' commands visible. The transmit packet queue holds two (or more?)
+ * packets: both 'retransmit this packet' (due to collision) and 'transmit next
+ * packet' commands must be started by hand.
+ *
+ * The station address is stored in a standard bit-serial EEPROM which must be
+ * read (ughh) by the device driver. (Provisions have been made for
+ * substituting a 74S288 PROM, but I haven't gotten reports of any models
+ * using it.) Unlike built-in devices, a pocket adapter can temporarily lose
+ * power without indication to the device driver. The major effect is that
+ * the station address, receive filter (promiscuous, etc.) and transceiver
+ * must be reset.
+ *
+ * The controller itself has 16 registers, some of which use only the lower
+ * bits. The registers are read and written 4 bits at a time. The four bit
+ * register address is presented on the data lines along with a few additional
+ * timing and control bits. The data is then read from status port or written
+ * to the data port.
+ *
+ * Correction: the controller has two banks of 16 registers. The second
+ * bank contains only the multicast filter table (now used) and the EEPROM
+ * access registers.
+ *
+ * Since the bulk data transfer of the actual packets through the slow
+ * parallel port dominates the driver's running time, four distinct data
+ * (non-register) transfer modes are provided by the adapter, two in each
+ * direction. In the first mode timing for the nibble transfers is
+ * provided through the data port. In the second mode the same timing is
+ * provided through the control port. In either case the data is read from
+ * the status port and written to the data port, just as it is accessing
+ * registers.
+ *
+ * In addition to the basic data transfer methods, several more are modes are
+ * created by adding some delay by doing multiple reads of the data to allow
+ * it to stabilize. This delay seems to be needed on most machines.
+ *
+ * The data transfer mode is stored in the 'dev->if_port' field. Its default
+ * value is '4'. It may be overridden at boot-time using the third parameter
+ * to the "ether=..." initialization.
+ *
+ * The header file <atp.h> provides inline functions that encapsulate the
+ * register and data access methods. These functions are hand-tuned to
+ * generate reasonable object code. This header file also documents my
+ * interpretations of the device registers.
+ */
#include <linux/kernel.h>
#include <linux/module.h>
@@ -140,7 +138,7 @@ static int xcvr[NUM_UNITS]; /* The data transfer mode. */
#include <linux/delay.h>
#include <linux/bitops.h>
-#include <asm/io.h>
+#include <linux/io.h>
#include <asm/dma.h>
#include "atp.h"
@@ -167,20 +165,21 @@ MODULE_PARM_DESC(xcvr, "ATP transceiver(s) (0=internal, 1=external)");
static char mux_8012[] = { 0xff, 0xf7, 0xff, 0xfb, 0xf3, 0xfb, 0xff, 0xf7,};
struct net_local {
- spinlock_t lock;
- struct net_device *next_module;
- struct timer_list timer; /* Media selection timer. */
- long last_rx_time; /* Last Rx, in jiffies, to handle Rx hang. */
- int saved_tx_size;
- unsigned int tx_unit_busy:1;
- unsigned char re_tx, /* Number of packet retransmissions. */
- addr_mode, /* Current Rx filter e.g. promiscuous, etc. */
+ spinlock_t lock;
+ struct net_device *next_module;
+ struct timer_list timer; /* Media selection timer. */
+ long last_rx_time; /* Last Rx, in jiffies, to handle Rx hang. */
+ int saved_tx_size;
+ unsigned int tx_unit_busy:1;
+ unsigned char re_tx, /* Number of packet retransmissions. */
+ addr_mode, /* Current Rx filter e.g. promiscuous, etc. */
pac_cnt_in_tx_buf;
};
/* This code, written by wwc@super.org, resets the adapter every
- TIMED_CHECKER ticks. This recovers from an unknown error which
- hangs the device. */
+ * TIMED_CHECKER ticks. This recovers from an unknown error which
+ * hangs the device.
+ */
#define TIMED_CHECKER (HZ/4)
#ifdef TIMED_CHECKER
#include <linux/timer.h>
@@ -194,41 +193,43 @@ static void get_node_ID(struct net_device *dev);
static unsigned short eeprom_op(long ioaddr, unsigned int cmd);
static int net_open(struct net_device *dev);
static void hardware_init(struct net_device *dev);
-static void write_packet(long ioaddr, int length, unsigned char *packet, int pad, int mode);
+static void write_packet(long ioaddr, int length, unsigned char *packet,
+ int pad, int mode);
static void trigger_send(long ioaddr, int length);
static netdev_tx_t atp_send_packet(struct sk_buff *skb,
struct net_device *dev);
static irqreturn_t atp_interrupt(int irq, void *dev_id);
static void net_rx(struct net_device *dev);
-static void read_block(long ioaddr, int length, unsigned char *buffer, int data_mode);
+static void read_block(long ioaddr, int length, unsigned char *buffer,
+ int data_mode);
static int net_close(struct net_device *dev);
static void set_rx_mode(struct net_device *dev);
static void tx_timeout(struct net_device *dev);
-
/* A list of all installed ATP devices, for removing the driver module. */
static struct net_device *root_atp_dev;
/* Check for a network adapter of this type, and return '0' iff one exists.
- If dev->base_addr == 0, probe all likely locations.
- If dev->base_addr == 1, always return failure.
- If dev->base_addr == 2, allocate space for the device and return success
- (detachable devices only).
-
- FIXME: we should use the parport layer for this
- */
+ * If dev->base_addr == 0, probe all likely locations.
+ * If dev->base_addr == 1, always return failure.
+ * If dev->base_addr == 2, allocate space for the device and return success
+ * (detachable devices only).
+ *
+ * FIXME: we should use the parport layer for this
+ */
static int __init atp_init(void)
{
int *port, ports[] = {0x378, 0x278, 0x3bc, 0};
int base_addr = io[0];
- if (base_addr > 0x1ff) /* Check a single specified location. */
+ if (base_addr > 0x1ff) /* Check a single specified location. */
return atp_probe1(base_addr);
else if (base_addr == 1) /* Don't probe at all. */
return -ENXIO;
for (port = ports; *port; port++) {
long ioaddr = *port;
+
outb(0x57, ioaddr + PAR_DATA);
if (inb(ioaddr + PAR_DATA) != 0x57)
continue;
@@ -246,7 +247,7 @@ static const struct net_device_ops atp_netdev_ops = {
.ndo_set_rx_mode = set_rx_mode,
.ndo_tx_timeout = tx_timeout,
.ndo_change_mtu = eth_change_mtu,
- .ndo_set_mac_address = eth_mac_addr,
+ .ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
};
@@ -259,7 +260,8 @@ static int __init atp_probe1(long ioaddr)
outb(0xff, ioaddr + PAR_DATA);
/* Save the original value of the Control register, in case we guessed
- wrong. */
+ * wrong.
+ */
saved_ctrl_reg = inb(ioaddr + PAR_CONTROL);
if (net_debug > 3)
printk("atp: Control register was %#2.2x.\n", saved_ctrl_reg);
@@ -330,8 +332,7 @@ static int __init atp_probe1(long ioaddr)
printk(KERN_INFO "%s", version);
#endif
- printk(KERN_NOTICE "%s: Pocket adapter found at %#3lx, IRQ %d, "
- "SAPROM %pM.\n",
+ printk(KERN_NOTICE "%s: Pocket adapter found at %#3lx, IRQ %d, SAPROM %pM.\n",
dev->name, dev->base_addr, dev->irq, dev->dev_addr);
/* Reset the ethernet hardware and activate the printer pass-through. */
@@ -349,7 +350,7 @@ static int __init atp_probe1(long ioaddr)
if (dev->mem_end & 0xf)
net_debug = dev->mem_end & 7;
- dev->netdev_ops = &atp_netdev_ops;
+ dev->netdev_ops = &atp_netdev_ops;
dev->watchdog_timeo = TX_TIMEOUT;
res = register_netdev(dev);
@@ -374,7 +375,8 @@ static void __init get_node_ID(struct net_device *dev)
write_reg(ioaddr, CMR2, CMR2_EEPROM); /* Point to the EEPROM control registers. */
/* Some adapters have the station address at offset 15 instead of offset
- zero. Check for it, and fix it if needed. */
+ * zero. Check for it, and fix it if needed.
+ */
if (eeprom_op(ioaddr, EE_READ(0)) == 0xffff)
sa_offset = 15;
@@ -385,9 +387,8 @@ static void __init get_node_ID(struct net_device *dev)
write_reg(ioaddr, CMR2, CMR2_NULL);
}
-/*
- An EEPROM read command starts by shifting out 0x60+address, and then
- shifting in the serial data. See the NatSemi databook for details.
+/* An EEPROM read command starts by shifting out 0x60+address, and then
+ * shifting in the serial data. See the NatSemi databook for details.
* ________________
* CS : __|
* ___ ___
@@ -404,6 +405,7 @@ static unsigned short __init eeprom_op(long ioaddr, u32 cmd)
while (--num_bits >= 0) {
char outval = (cmd & (1<<num_bits)) ? EE_DATA_WRITE : 0;
+
write_reg_high(ioaddr, PROM_CMD, outval | EE_CLK_LOW);
write_reg_high(ioaddr, PROM_CMD, outval | EE_CLK_HIGH);
eedata_out <<= 1;
@@ -414,25 +416,25 @@ static unsigned short __init eeprom_op(long ioaddr, u32 cmd)
return eedata_out;
}
-
/* Open/initialize the board. This is called (in the current kernel)
- sometime after booting when the 'ifconfig' program is run.
-
- This routine sets everything up anew at each open, even
- registers that "should" only need to be set once at boot, so that
- there is non-reboot way to recover if something goes wrong.
-
- This is an attachable device: if there is no private entry then it wasn't
- probed for at boot-time, and we need to probe for it again.
- */
+ * sometime after booting when the 'ifconfig' program is run.
+ *
+ * This routine sets everything up anew at each open, even
+ * registers that "should" only need to be set once at boot, so that
+ * there is non-reboot way to recover if something goes wrong.
+ *
+ * This is an attachable device: if there is no private entry then it wasn't
+ * probed for at boot-time, and we need to probe for it again.
+ */
static int net_open(struct net_device *dev)
{
struct net_local *lp = netdev_priv(dev);
int ret;
/* The interrupt line is turned off (tri-stated) when the device isn't in
- use. That's especially important for "attached" interfaces where the
- port or interrupt may be shared. */
+ * use. That's especially important for "attached" interfaces where the
+ * port or interrupt may be shared.
+ */
ret = request_irq(dev->irq, atp_interrupt, 0, dev->name, dev);
if (ret)
return ret;
@@ -450,40 +452,41 @@ static int net_open(struct net_device *dev)
}
/* This routine resets the hardware. We initialize everything, assuming that
- the hardware may have been temporarily detached. */
+ * the hardware may have been temporarily detached.
+ */
static void hardware_init(struct net_device *dev)
{
struct net_local *lp = netdev_priv(dev);
long ioaddr = dev->base_addr;
- int i;
+ int i;
/* Turn off the printer multiplexer on the 8012. */
for (i = 0; i < 8; i++)
outb(mux_8012[i], ioaddr + PAR_DATA);
write_reg_high(ioaddr, CMR1, CMR1h_RESET);
- for (i = 0; i < 6; i++)
+ for (i = 0; i < 6; i++)
write_reg_byte(ioaddr, PAR0 + i, dev->dev_addr[i]);
write_reg_high(ioaddr, CMR2, lp->addr_mode);
if (net_debug > 2) {
printk(KERN_DEBUG "%s: Reset: current Rx mode %d.\n", dev->name,
- (read_nibble(ioaddr, CMR2_h) >> 3) & 0x0f);
+ (read_nibble(ioaddr, CMR2_h) >> 3) & 0x0f);
}
- write_reg(ioaddr, CMR2, CMR2_IRQOUT);
- write_reg_high(ioaddr, CMR1, CMR1h_RxENABLE | CMR1h_TxENABLE);
+ write_reg(ioaddr, CMR2, CMR2_IRQOUT);
+ write_reg_high(ioaddr, CMR1, CMR1h_RxENABLE | CMR1h_TxENABLE);
/* Enable the interrupt line from the serial port. */
outb(Ctrl_SelData + Ctrl_IRQEN, ioaddr + PAR_CONTROL);
/* Unmask the interesting interrupts. */
- write_reg(ioaddr, IMR, ISR_RxOK | ISR_TxErr | ISR_TxOK);
- write_reg_high(ioaddr, IMR, ISRh_RxErr);
+ write_reg(ioaddr, IMR, ISR_RxOK | ISR_TxErr | ISR_TxOK);
+ write_reg_high(ioaddr, IMR, ISRh_RxErr);
lp->tx_unit_busy = 0;
- lp->pac_cnt_in_tx_buf = 0;
+ lp->pac_cnt_in_tx_buf = 0;
lp->saved_tx_size = 0;
}
@@ -496,23 +499,22 @@ static void trigger_send(long ioaddr, int length)
static void write_packet(long ioaddr, int length, unsigned char *packet, int pad_len, int data_mode)
{
- if (length & 1)
- {
- length++;
- pad_len++;
- }
-
- outb(EOC+MAR, ioaddr + PAR_DATA);
- if ((data_mode & 1) == 0) {
+ if (length & 1) {
+ length++;
+ pad_len++;
+ }
+
+ outb(EOC+MAR, ioaddr + PAR_DATA);
+ if ((data_mode & 1) == 0) {
/* Write the packet out, starting with the write addr. */
outb(WrAddr+MAR, ioaddr + PAR_DATA);
do {
write_byte_mode0(ioaddr, *packet++);
- } while (--length > pad_len) ;
+ } while (--length > pad_len);
do {
write_byte_mode0(ioaddr, 0);
- } while (--length > 0) ;
- } else {
+ } while (--length > 0);
+ } else {
/* Write the packet out in slow mode. */
unsigned char outbyte = *packet++;
@@ -528,10 +530,10 @@ static void write_packet(long ioaddr, int length, unsigned char *packet, int pad
write_byte_mode1(ioaddr, *packet++);
while (--length > 0)
write_byte_mode1(ioaddr, 0);
- }
- /* Terminate the Tx frame. End of write: ECB. */
- outb(0xff, ioaddr + PAR_DATA);
- outb(Ctrl_HNibWrite | Ctrl_SelData | Ctrl_IRQEN, ioaddr + PAR_CONTROL);
+ }
+ /* Terminate the Tx frame. End of write: ECB. */
+ outb(0xff, ioaddr + PAR_DATA);
+ outb(Ctrl_HNibWrite | Ctrl_SelData | Ctrl_IRQEN, ioaddr + PAR_CONTROL);
}
static void tx_timeout(struct net_device *dev)
@@ -539,8 +541,8 @@ static void tx_timeout(struct net_device *dev)
long ioaddr = dev->base_addr;
printk(KERN_WARNING "%s: Transmit timed out, %s?\n", dev->name,
- inb(ioaddr + PAR_CONTROL) & 0x10 ? "network cable problem"
- : "IRQ conflict");
+ inb(ioaddr + PAR_CONTROL) & 0x10 ? "network cable problem"
+ : "IRQ conflict");
dev->stats.tx_errors++;
/* Try to restart the adapter. */
hardware_init(dev);
@@ -562,7 +564,8 @@ static netdev_tx_t atp_send_packet(struct sk_buff *skb,
netif_stop_queue(dev);
/* Disable interrupts by writing 0x00 to the Interrupt Mask Register.
- This sequence must not be interrupted by an incoming packet. */
+ * This sequence must not be interrupted by an incoming packet.
+ */
spin_lock_irqsave(&lp->lock, flags);
write_reg(ioaddr, IMR, 0);
@@ -574,22 +577,23 @@ static netdev_tx_t atp_send_packet(struct sk_buff *skb,
lp->pac_cnt_in_tx_buf++;
if (lp->tx_unit_busy == 0) {
trigger_send(ioaddr, length);
- lp->saved_tx_size = 0; /* Redundant */
+ lp->saved_tx_size = 0; /* Redundant */
lp->re_tx = 0;
lp->tx_unit_busy = 1;
- } else
+ } else {
lp->saved_tx_size = length;
+ }
/* Re-enable the LPT interrupts. */
write_reg(ioaddr, IMR, ISR_RxOK | ISR_TxErr | ISR_TxOK);
write_reg_high(ioaddr, IMR, ISRh_RxErr);
- dev_kfree_skb (skb);
+ dev_kfree_skb(skb);
return NETDEV_TX_OK;
}
-
/* The typical workload of the driver:
- Handle the network interface interrupts. */
+ * Handle the network interface interrupts.
+ */
static irqreturn_t atp_interrupt(int irq, void *dev_instance)
{
struct net_device *dev = dev_instance;
@@ -611,20 +615,25 @@ static irqreturn_t atp_interrupt(int irq, void *dev_instance)
write_reg(ioaddr, CMR2, CMR2_NULL);
write_reg(ioaddr, IMR, 0);
- if (net_debug > 5) printk(KERN_DEBUG "%s: In interrupt ", dev->name);
- while (--boguscount > 0) {
+ if (net_debug > 5)
+ printk(KERN_DEBUG "%s: In interrupt ", dev->name);
+ while (--boguscount > 0) {
int status = read_nibble(ioaddr, ISR);
- if (net_debug > 5) printk("loop status %02x..", status);
+
+ if (net_debug > 5)
+ printk("loop status %02x..", status);
if (status & (ISR_RxOK<<3)) {
handled = 1;
write_reg(ioaddr, ISR, ISR_RxOK); /* Clear the Rx interrupt. */
do {
int read_status = read_nibble(ioaddr, CMR1);
+
if (net_debug > 6)
printk("handling Rx packet %02x..", read_status);
/* We acknowledged the normal Rx interrupt, so if the interrupt
- is still outstanding we must have a Rx error. */
+ * is still outstanding we must have a Rx error.
+ */
if (read_status & (CMR1_IRQ << 3)) { /* Overrun. */
dev->stats.rx_over_errors++;
/* Set to no-accept mode long enough to remove a packet. */
@@ -636,14 +645,17 @@ static irqreturn_t atp_interrupt(int irq, void *dev_instance)
} else if ((read_status & (CMR1_BufEnb << 3)) == 0) {
net_rx(dev);
num_tx_since_rx = 0;
- } else
+ } else {
break;
+ }
} while (--boguscount > 0);
} else if (status & ((ISR_TxErr + ISR_TxOK)<<3)) {
handled = 1;
- if (net_debug > 6) printk("handling Tx done..");
- /* Clear the Tx interrupt. We should check for too many failures
- and reinitialize the adapter. */
+ if (net_debug > 6)
+ printk("handling Tx done..");
+ /* Clear the Tx interrupt. We should check for too many
+ * failures and reinitialize the adapter.
+ */
write_reg(ioaddr, ISR, ISR_TxErr + ISR_TxOK);
if (status & (ISR_TxErr<<3)) {
dev->stats.collisions++;
@@ -653,38 +665,41 @@ static irqreturn_t atp_interrupt(int irq, void *dev_instance)
break;
}
/* Attempt to retransmit. */
- if (net_debug > 6) printk("attempting to ReTx");
+ if (net_debug > 6)
+ printk("attempting to ReTx");
write_reg(ioaddr, CMR1, CMR1_ReXmit + CMR1_Xmit);
} else {
/* Finish up the transmit. */
dev->stats.tx_packets++;
lp->pac_cnt_in_tx_buf--;
- if ( lp->saved_tx_size) {
+ if (lp->saved_tx_size) {
trigger_send(ioaddr, lp->saved_tx_size);
lp->saved_tx_size = 0;
lp->re_tx = 0;
- } else
+ } else {
lp->tx_unit_busy = 0;
+ }
netif_wake_queue(dev); /* Inform upper layers. */
}
num_tx_since_rx++;
} else if (num_tx_since_rx > 8 &&
time_after(jiffies, dev->last_rx + HZ)) {
if (net_debug > 2)
- printk(KERN_DEBUG "%s: Missed packet? No Rx after %d Tx and "
- "%ld jiffies status %02x CMR1 %02x.\n", dev->name,
- num_tx_since_rx, jiffies - dev->last_rx, status,
- (read_nibble(ioaddr, CMR1) >> 3) & 15);
+ printk(KERN_DEBUG "%s: Missed packet? No Rx after %d Tx and %ld jiffies status %02x CMR1 %02x.\n",
+ dev->name, num_tx_since_rx, jiffies - dev->last_rx, status,
+ (read_nibble(ioaddr, CMR1) >> 3) & 15);
dev->stats.rx_missed_errors++;
hardware_init(dev);
num_tx_since_rx = 0;
break;
- } else
+ } else {
break;
- }
+ }
+ }
/* This following code fixes a rare (and very difficult to track down)
- problem where the adapter forgets its ethernet address. */
+ * problem where the adapter forgets its ethernet address.
+ */
{
int i;
for (i = 0; i < 6; i++)
@@ -695,22 +710,24 @@ static irqreturn_t atp_interrupt(int irq, void *dev_instance)
}
/* Tell the adapter that it can go back to using the output line as IRQ. */
- write_reg(ioaddr, CMR2, CMR2_IRQOUT);
+ write_reg(ioaddr, CMR2, CMR2_IRQOUT);
/* Enable the physical interrupt line, which is sure to be low until.. */
outb(Ctrl_SelData + Ctrl_IRQEN, ioaddr + PAR_CONTROL);
/* .. we enable the interrupt sources. */
write_reg(ioaddr, IMR, ISR_RxOK | ISR_TxErr | ISR_TxOK);
- write_reg_high(ioaddr, IMR, ISRh_RxErr); /* Hmmm, really needed? */
+ write_reg_high(ioaddr, IMR, ISRh_RxErr); /* Hmmm, really needed? */
spin_unlock(&lp->lock);
- if (net_debug > 5) printk("exiting interrupt.\n");
+ if (net_debug > 5)
+ printk("exiting interrupt.\n");
return IRQ_RETVAL(handled);
}
#ifdef TIMED_CHECKER
/* This following code fixes a rare (and very difficult to track down)
- problem where the adapter forgets its ethernet address. */
+ * problem where the adapter forgets its ethernet address.
+ */
static void atp_timed_checker(unsigned long data)
{
struct net_device *dev = (struct net_device *)data;
@@ -727,19 +744,19 @@ static void atp_timed_checker(unsigned long data)
lp->last_rx_time = jiffies;
#else
for (i = 0; i < 6; i++)
- if (read_cmd_byte(ioaddr, PAR0 + i) != atp_timed_dev->dev_addr[i])
- {
- struct net_local *lp = netdev_priv(atp_timed_dev);
- write_reg_byte(ioaddr, PAR0 + i, atp_timed_dev->dev_addr[i]);
- if (i == 2)
- dev->stats.tx_errors++;
- else if (i == 3)
- dev->stats.tx_dropped++;
- else if (i == 4)
- dev->stats.collisions++;
- else
- dev->stats.rx_errors++;
- }
+ if (read_cmd_byte(ioaddr, PAR0 + i) != atp_timed_dev->dev_addr[i]) {
+ struct net_local *lp = netdev_priv(atp_timed_dev);
+
+ write_reg_byte(ioaddr, PAR0 + i, atp_timed_dev->dev_addr[i]);
+ if (i == 2)
+ dev->stats.tx_errors++;
+ else if (i == 3)
+ dev->stats.tx_dropped++;
+ else if (i == 4)
+ dev->stats.collisions++;
+ else
+ dev->stats.rx_errors++;
+ }
#endif
}
spin_unlock(&lp->lock);
@@ -757,23 +774,26 @@ static void net_rx(struct net_device *dev)
/* Process the received packet. */
outb(EOC+MAR, ioaddr + PAR_DATA);
- read_block(ioaddr, 8, (unsigned char*)&rx_head, dev->if_port);
+ read_block(ioaddr, 8, (unsigned char *)&rx_head, dev->if_port);
if (net_debug > 5)
printk(KERN_DEBUG " rx_count %04x %04x %04x %04x..", rx_head.pad,
- rx_head.rx_count, rx_head.rx_status, rx_head.cur_addr);
+ rx_head.rx_count, rx_head.rx_status, rx_head.cur_addr);
if ((rx_head.rx_status & 0x77) != 0x01) {
dev->stats.rx_errors++;
- if (rx_head.rx_status & 0x0004) dev->stats.rx_frame_errors++;
- else if (rx_head.rx_status & 0x0002) dev->stats.rx_crc_errors++;
+ if (rx_head.rx_status & 0x0004)
+ dev->stats.rx_frame_errors++;
+ else if (rx_head.rx_status & 0x0002)
+ dev->stats.rx_crc_errors++;
if (net_debug > 3)
printk(KERN_DEBUG "%s: Unknown ATP Rx error %04x.\n",
- dev->name, rx_head.rx_status);
+ dev->name, rx_head.rx_status);
if (rx_head.rx_status & 0x0020) {
dev->stats.rx_fifo_errors++;
write_reg_high(ioaddr, CMR1, CMR1h_TxENABLE);
write_reg_high(ioaddr, CMR1, CMR1h_RxENABLE | CMR1h_TxENABLE);
- } else if (rx_head.rx_status & 0x0050)
+ } else if (rx_head.rx_status & 0x0050) {
hardware_init(dev);
+ }
return;
} else {
/* Malloc up new buffer. The "-4" omits the FCS (CRC). */
@@ -787,7 +807,7 @@ static void net_rx(struct net_device *dev)
}
skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
- read_block(ioaddr, pkt_len, skb_put(skb,pkt_len), dev->if_port);
+ read_block(ioaddr, pkt_len, skb_put(skb, pkt_len), dev->if_port);
skb->protocol = eth_type_trans(skb, dev);
netif_rx(skb);
dev->last_rx = jiffies;
@@ -804,7 +824,7 @@ static void read_block(long ioaddr, int length, unsigned char *p, int data_mode)
if (data_mode <= 3) { /* Mode 0 or 1 */
outb(Ctrl_LNibRead, ioaddr + PAR_CONTROL);
outb(length == 8 ? RdAddr | HNib | MAR : RdAddr | MAR,
- ioaddr + PAR_DATA);
+ ioaddr + PAR_DATA);
if (data_mode <= 1) { /* Mode 0 or 1 */
do { *p++ = read_byte_mode0(ioaddr); } while (--length > 0);
} else { /* Mode 2 or 3 */
@@ -844,8 +864,7 @@ net_close(struct net_device *dev)
return 0;
}
-/*
- * Set or clear the multicast filter for this adapter.
+/* Set or clear the multicast filter for this adapter.
*/
static void set_rx_mode(struct net_device *dev)
@@ -860,17 +879,20 @@ static void set_rx_mode(struct net_device *dev)
write_reg_high(ioaddr, CMR2, lp->addr_mode);
}
-static int __init atp_init_module(void) {
- if (debug) /* Emit version even if no cards detected. */
+static int __init atp_init_module(void)
+{
+ if (debug) /* Emit version even if no cards detected. */
printk(KERN_INFO "%s", version);
return atp_init();
}
-static void __exit atp_cleanup_module(void) {
+static void __exit atp_cleanup_module(void)
+{
struct net_device *next_dev;
while (root_atp_dev) {
struct net_local *atp_local = netdev_priv(root_atp_dev);
+
next_dev = atp_local->next_module;
unregister_netdev(root_atp_dev);
/* No need to release_region(), since we never snarf it. */
--
2.1.3
^ permalink raw reply related
* Re: [Xen-devel] [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: Zoltan Kiss @ 2014-11-03 18:23 UTC (permalink / raw)
To: David Vrabel, Ian Campbell; +Cc: netdev, Malcolm Crossley, Wei Liu, xen-devel
In-Reply-To: <5457BF80.2000205@citrix.com>
On 03/11/14 17:46, David Vrabel wrote:
> On 03/11/14 17:39, Ian Campbell wrote:
>> On Mon, 2014-11-03 at 17:23 +0000, David Vrabel wrote:
>>> From: Malcolm Crossley <malcolm.crossley@citrix.com>
>>>
>>> Unconditionally pulling 128 bytes into the linear buffer is not
>>> required. Netback has already grant copied up-to 128 bytes from the
>>> first slot of a packet into the linear buffer. The first slot normally
>>> contain all the IPv4/IPv6 and TCP/UDP headers.
>>
>> What about when it doesn't? It sounds as if we now won't pull up, which
>> would be bad.
>
> The network stack will always pull any headers it needs to inspect (the
> frag may be a userspace page which has the same security issues as a
> frag with a foreign page).
I wouldn't bet my life on this, but indeed it should always happen.
>
> e.g., see skb_checksum_setup() called slightly later on in netback.
Fortunately the call to checksum_setup a bit later makes sure that at
least the TCP/UDP headers are in the linear area, which is quite the
same as blindly pulling 128 bytes. With any other protocol we rely on
the network stack that it will enforce packet header in the linear buffer.
Regards,
Zoli
^ permalink raw reply
* Re: [PATCH] VNIC: Adding support for Cavium ThunderX network controller
From: Stephen Hemminger @ 2014-11-03 18:16 UTC (permalink / raw)
To: Sunil Kovvuri
Cc: Robert Richter, David S. Miller, Sunil Goutham, Robert Richter,
Stefan Assmann, LKML, LAKML, netdev
In-Reply-To: <CA+sq2CfV-90CRvo-FCizkzoxHq-AsfOEDW2Z5pXtnNLNXCfTUg@mail.gmail.com>
On Fri, 31 Oct 2014 22:44:11 +0530
Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Fri, Oct 31, 2014 at 8:24 AM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > On Thu, 30 Oct 2014 17:54:34 +0100
> > Robert Richter <rric@kernel.org> wrote:
> >
> >> +#ifdef VNIC_RSS_SUPPORT
> >> +static int rss_config = RSS_IP_HASH_ENA | RSS_TCP_HASH_ENA | RSS_UDP_HASH_ENA;
> >> +module_param(rss_config, int, S_IRUGO);
> >> +MODULE_PARM_DESC(rss_config,
> >> + "RSS hash config [bits 8:0] (Bit0:L2 extended, 1:IP, 2:TCP, 3:TCP SYN, 4:UDP, 5:L4 extended, 6:ROCE 7:L3 bi-directional, 8:L4 bi-directional)");
> >> +#endif
> >
> > This should managed be via ethtool ETHTOOL_GRXFH rather than a module parameter.
> Thanks, i will add setting hash options via ETHTOOL_SRXFH as well.
> The idea here is to have a choice of hash while module load (through
> module params) and if it needs to be changed runtime then
> via Ethtool.
>
> Sunil.
Network developers do not like vendor unique module parameters.
Anything device specific doesn't work in a generic distro environment.
^ permalink raw reply
* [Patch net-next v2] neigh: remove dynamic neigh table registration support
From: Cong Wang @ 2014-11-03 18:14 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Cong Wang
Currently there are only three neigh tables in the whole kernel:
arp table, ndisc table and decnet neigh table. What's more,
we don't support registering multiple tables per family.
Therefore we can just make these tables statically built-in.
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
v2: remove useless #ifdef's
move the assignment to the end of neigh_table_init()
include/net/neighbour.h | 9 +-
net/core/neighbour.c | 244 ++++++++++++++++++++++--------------------------
net/decnet/dn_neigh.c | 2 +-
net/ipv4/arp.c | 2 +-
net/ipv6/ndisc.c | 2 +-
5 files changed, 121 insertions(+), 138 deletions(-)
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index dedfb18..1e65c68 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -220,6 +220,13 @@ struct neigh_table {
struct pneigh_entry **phash_buckets;
};
+enum {
+ NEIGH_ARP_TABLE = 0,
+ NEIGH_ND_TABLE = 1,
+ NEIGH_DN_TABLE = 2,
+ NEIGH_NR_TABLES,
+};
+
static inline int neigh_parms_family(struct neigh_parms *p)
{
return p->tbl->family;
@@ -240,7 +247,7 @@ static inline void *neighbour_priv(const struct neighbour *n)
#define NEIGH_UPDATE_F_ISROUTER 0x40000000
#define NEIGH_UPDATE_F_ADMIN 0x80000000
-void neigh_table_init(struct neigh_table *tbl);
+void neigh_table_init(int index, struct neigh_table *tbl);
int neigh_table_clear(struct neigh_table *tbl);
struct neighbour *neigh_lookup(struct neigh_table *tbl, const void *pkey,
struct net_device *dev);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index edd0411..55b7e0b 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -56,7 +56,6 @@ static void __neigh_notify(struct neighbour *n, int type, int flags);
static void neigh_update_notify(struct neighbour *neigh);
static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
-static struct neigh_table *neigh_tables;
#ifdef CONFIG_PROC_FS
static const struct file_operations neigh_stat_seq_fops;
#endif
@@ -87,13 +86,8 @@ static const struct file_operations neigh_stat_seq_fops;
the most complicated procedure, which we allow is dev->hard_header.
It is supposed, that dev->hard_header is simplistic and does
not make callbacks to neighbour tables.
-
- The last lock is neigh_tbl_lock. It is pure SMP lock, protecting
- list of neighbour tables. This list is used only in process context,
*/
-static DEFINE_RWLOCK(neigh_tbl_lock);
-
static int neigh_blackhole(struct neighbour *neigh, struct sk_buff *skb)
{
kfree_skb(skb);
@@ -1520,7 +1514,9 @@ static void neigh_parms_destroy(struct neigh_parms *parms)
static struct lock_class_key neigh_table_proxy_queue_class;
-static void neigh_table_init_no_netlink(struct neigh_table *tbl)
+static struct neigh_table *neigh_tables[NEIGH_NR_TABLES] __read_mostly;
+
+void neigh_table_init(int index, struct neigh_table *tbl)
{
unsigned long now = jiffies;
unsigned long phsize;
@@ -1566,34 +1562,13 @@ static void neigh_table_init_no_netlink(struct neigh_table *tbl)
tbl->last_flush = now;
tbl->last_rand = now + tbl->parms.reachable_time * 20;
-}
-
-void neigh_table_init(struct neigh_table *tbl)
-{
- struct neigh_table *tmp;
- neigh_table_init_no_netlink(tbl);
- write_lock(&neigh_tbl_lock);
- for (tmp = neigh_tables; tmp; tmp = tmp->next) {
- if (tmp->family == tbl->family)
- break;
- }
- tbl->next = neigh_tables;
- neigh_tables = tbl;
- write_unlock(&neigh_tbl_lock);
-
- if (unlikely(tmp)) {
- pr_err("Registering multiple tables for family %d\n",
- tbl->family);
- dump_stack();
- }
+ neigh_tables[index] = tbl;
}
EXPORT_SYMBOL(neigh_table_init);
int neigh_table_clear(struct neigh_table *tbl)
{
- struct neigh_table **tp;
-
/* It is not clean... Fix it to unload IPv6 module safely */
cancel_delayed_work_sync(&tbl->gc_work);
del_timer_sync(&tbl->proxy_timer);
@@ -1601,14 +1576,6 @@ int neigh_table_clear(struct neigh_table *tbl)
neigh_ifdown(tbl, NULL);
if (atomic_read(&tbl->entries))
pr_crit("neighbour leakage\n");
- write_lock(&neigh_tbl_lock);
- for (tp = &neigh_tables; *tp; tp = &(*tp)->next) {
- if (*tp == tbl) {
- *tp = tbl->next;
- break;
- }
- }
- write_unlock(&neigh_tbl_lock);
call_rcu(&rcu_dereference_protected(tbl->nht, 1)->rcu,
neigh_hash_free_rcu);
@@ -1626,12 +1593,32 @@ int neigh_table_clear(struct neigh_table *tbl)
}
EXPORT_SYMBOL(neigh_table_clear);
+static struct neigh_table *neigh_find_table(int family)
+{
+ struct neigh_table *tbl = NULL;
+
+ switch (family) {
+ case AF_INET:
+ tbl = neigh_tables[NEIGH_ARP_TABLE];
+ break;
+ case AF_INET6:
+ tbl = neigh_tables[NEIGH_ND_TABLE];
+ break;
+ case AF_DECnet:
+ tbl = neigh_tables[NEIGH_DN_TABLE];
+ break;
+ }
+
+ return tbl;
+}
+
static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct net *net = sock_net(skb->sk);
struct ndmsg *ndm;
struct nlattr *dst_attr;
struct neigh_table *tbl;
+ struct neighbour *neigh;
struct net_device *dev = NULL;
int err = -EINVAL;
@@ -1652,39 +1639,31 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh)
}
}
- read_lock(&neigh_tbl_lock);
- for (tbl = neigh_tables; tbl; tbl = tbl->next) {
- struct neighbour *neigh;
-
- if (tbl->family != ndm->ndm_family)
- continue;
- read_unlock(&neigh_tbl_lock);
-
- if (nla_len(dst_attr) < tbl->key_len)
- goto out;
+ tbl = neigh_find_table(ndm->ndm_family);
+ if (tbl == NULL)
+ return -EAFNOSUPPORT;
- if (ndm->ndm_flags & NTF_PROXY) {
- err = pneigh_delete(tbl, net, nla_data(dst_attr), dev);
- goto out;
- }
+ if (nla_len(dst_attr) < tbl->key_len)
+ goto out;
- if (dev == NULL)
- goto out;
+ if (ndm->ndm_flags & NTF_PROXY) {
+ err = pneigh_delete(tbl, net, nla_data(dst_attr), dev);
+ goto out;
+ }
- neigh = neigh_lookup(tbl, nla_data(dst_attr), dev);
- if (neigh == NULL) {
- err = -ENOENT;
- goto out;
- }
+ if (dev == NULL)
+ goto out;
- err = neigh_update(neigh, NULL, NUD_FAILED,
- NEIGH_UPDATE_F_OVERRIDE |
- NEIGH_UPDATE_F_ADMIN);
- neigh_release(neigh);
+ neigh = neigh_lookup(tbl, nla_data(dst_attr), dev);
+ if (neigh == NULL) {
+ err = -ENOENT;
goto out;
}
- read_unlock(&neigh_tbl_lock);
- err = -EAFNOSUPPORT;
+
+ err = neigh_update(neigh, NULL, NUD_FAILED,
+ NEIGH_UPDATE_F_OVERRIDE |
+ NEIGH_UPDATE_F_ADMIN);
+ neigh_release(neigh);
out:
return err;
@@ -1692,11 +1671,14 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh)
static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh)
{
+ int flags = NEIGH_UPDATE_F_ADMIN | NEIGH_UPDATE_F_OVERRIDE;
struct net *net = sock_net(skb->sk);
struct ndmsg *ndm;
struct nlattr *tb[NDA_MAX+1];
struct neigh_table *tbl;
struct net_device *dev = NULL;
+ struct neighbour *neigh;
+ void *dst, *lladdr;
int err;
ASSERT_RTNL();
@@ -1720,70 +1702,60 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh)
goto out;
}
- read_lock(&neigh_tbl_lock);
- for (tbl = neigh_tables; tbl; tbl = tbl->next) {
- int flags = NEIGH_UPDATE_F_ADMIN | NEIGH_UPDATE_F_OVERRIDE;
- struct neighbour *neigh;
- void *dst, *lladdr;
+ tbl = neigh_find_table(ndm->ndm_family);
+ if (tbl == NULL)
+ return -EAFNOSUPPORT;
- if (tbl->family != ndm->ndm_family)
- continue;
- read_unlock(&neigh_tbl_lock);
+ if (nla_len(tb[NDA_DST]) < tbl->key_len)
+ goto out;
+ dst = nla_data(tb[NDA_DST]);
+ lladdr = tb[NDA_LLADDR] ? nla_data(tb[NDA_LLADDR]) : NULL;
- if (nla_len(tb[NDA_DST]) < tbl->key_len)
- goto out;
- dst = nla_data(tb[NDA_DST]);
- lladdr = tb[NDA_LLADDR] ? nla_data(tb[NDA_LLADDR]) : NULL;
+ if (ndm->ndm_flags & NTF_PROXY) {
+ struct pneigh_entry *pn;
+
+ err = -ENOBUFS;
+ pn = pneigh_lookup(tbl, net, dst, dev, 1);
+ if (pn) {
+ pn->flags = ndm->ndm_flags;
+ err = 0;
+ }
+ goto out;
+ }
- if (ndm->ndm_flags & NTF_PROXY) {
- struct pneigh_entry *pn;
+ if (dev == NULL)
+ goto out;
- err = -ENOBUFS;
- pn = pneigh_lookup(tbl, net, dst, dev, 1);
- if (pn) {
- pn->flags = ndm->ndm_flags;
- err = 0;
- }
+ neigh = neigh_lookup(tbl, dst, dev);
+ if (neigh == NULL) {
+ if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
+ err = -ENOENT;
goto out;
}
- if (dev == NULL)
+ neigh = __neigh_lookup_errno(tbl, dst, dev);
+ if (IS_ERR(neigh)) {
+ err = PTR_ERR(neigh);
+ goto out;
+ }
+ } else {
+ if (nlh->nlmsg_flags & NLM_F_EXCL) {
+ err = -EEXIST;
+ neigh_release(neigh);
goto out;
-
- neigh = neigh_lookup(tbl, dst, dev);
- if (neigh == NULL) {
- if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
- err = -ENOENT;
- goto out;
- }
-
- neigh = __neigh_lookup_errno(tbl, dst, dev);
- if (IS_ERR(neigh)) {
- err = PTR_ERR(neigh);
- goto out;
- }
- } else {
- if (nlh->nlmsg_flags & NLM_F_EXCL) {
- err = -EEXIST;
- neigh_release(neigh);
- goto out;
- }
-
- if (!(nlh->nlmsg_flags & NLM_F_REPLACE))
- flags &= ~NEIGH_UPDATE_F_OVERRIDE;
}
- if (ndm->ndm_flags & NTF_USE) {
- neigh_event_send(neigh, NULL);
- err = 0;
- } else
- err = neigh_update(neigh, lladdr, ndm->ndm_state, flags);
- neigh_release(neigh);
- goto out;
+ if (!(nlh->nlmsg_flags & NLM_F_REPLACE))
+ flags &= ~NEIGH_UPDATE_F_OVERRIDE;
}
- read_unlock(&neigh_tbl_lock);
- err = -EAFNOSUPPORT;
+ if (ndm->ndm_flags & NTF_USE) {
+ neigh_event_send(neigh, NULL);
+ err = 0;
+ } else
+ err = neigh_update(neigh, lladdr, ndm->ndm_state, flags);
+ neigh_release(neigh);
+
out:
return err;
}
@@ -1982,7 +1954,8 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh)
struct neigh_table *tbl;
struct ndtmsg *ndtmsg;
struct nlattr *tb[NDTA_MAX+1];
- int err;
+ bool found = false;
+ int err, tidx;
err = nlmsg_parse(nlh, sizeof(*ndtmsg), tb, NDTA_MAX,
nl_neightbl_policy);
@@ -1995,19 +1968,21 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh)
}
ndtmsg = nlmsg_data(nlh);
- read_lock(&neigh_tbl_lock);
- for (tbl = neigh_tables; tbl; tbl = tbl->next) {
+
+ for (tidx = 0; tidx < NEIGH_NR_TABLES; tidx++) {
+ tbl = neigh_tables[tidx];
+ if (!tbl)
+ continue;
if (ndtmsg->ndtm_family && tbl->family != ndtmsg->ndtm_family)
continue;
-
- if (nla_strcmp(tb[NDTA_NAME], tbl->id) == 0)
+ if (nla_strcmp(tb[NDTA_NAME], tbl->id) == 0) {
+ found = true;
break;
+ }
}
- if (tbl == NULL) {
- err = -ENOENT;
- goto errout_locked;
- }
+ if (!found)
+ return -ENOENT;
/*
* We acquire tbl->lock to be nice to the periodic timers and
@@ -2118,8 +2093,6 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh)
errout_tbl_lock:
write_unlock_bh(&tbl->lock);
-errout_locked:
- read_unlock(&neigh_tbl_lock);
errout:
return err;
}
@@ -2134,10 +2107,13 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
- read_lock(&neigh_tbl_lock);
- for (tbl = neigh_tables, tidx = 0; tbl; tbl = tbl->next, tidx++) {
+ for (tidx = 0; tidx < NEIGH_NR_TABLES; tidx++) {
struct neigh_parms *p;
+ tbl = neigh_tables[tidx];
+ if (!tbl)
+ continue;
+
if (tidx < tbl_skip || (family && tbl->family != family))
continue;
@@ -2168,7 +2144,6 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
neigh_skip = 0;
}
out:
- read_unlock(&neigh_tbl_lock);
cb->args[0] = tidx;
cb->args[1] = nidx;
@@ -2351,7 +2326,6 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
int proxy = 0;
int err;
- read_lock(&neigh_tbl_lock);
family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
/* check for full ndmsg structure presence, family member is
@@ -2363,8 +2337,11 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
s_t = cb->args[0];
- for (tbl = neigh_tables, t = 0; tbl;
- tbl = tbl->next, t++) {
+ for (t = 0; t < NEIGH_NR_TABLES; t++) {
+ tbl = neigh_tables[t];
+
+ if (!tbl)
+ continue;
if (t < s_t || (family && tbl->family != family))
continue;
if (t > s_t)
@@ -2377,7 +2354,6 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
if (err < 0)
break;
}
- read_unlock(&neigh_tbl_lock);
cb->args[0] = t;
return skb->len;
diff --git a/net/decnet/dn_neigh.c b/net/decnet/dn_neigh.c
index c8121ce..845957a 100644
--- a/net/decnet/dn_neigh.c
+++ b/net/decnet/dn_neigh.c
@@ -591,7 +591,7 @@ static const struct file_operations dn_neigh_seq_fops = {
void __init dn_neigh_init(void)
{
- neigh_table_init(&dn_neigh_table);
+ neigh_table_init(NEIGH_DN_TABLE, &dn_neigh_table);
proc_create("decnet_neigh", S_IRUGO, init_net.proc_net,
&dn_neigh_seq_fops);
}
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 16acb59..205e147 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1292,7 +1292,7 @@ static int arp_proc_init(void);
void __init arp_init(void)
{
- neigh_table_init(&arp_tbl);
+ neigh_table_init(NEIGH_ARP_TABLE, &arp_tbl);
dev_add_pack(&arp_packet_type);
arp_proc_init();
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 4cb45c1..c67f339 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1763,7 +1763,7 @@ int __init ndisc_init(void)
/*
* Initialize the neighbour table
*/
- neigh_table_init(&nd_tbl);
+ neigh_table_init(NEIGH_ND_TABLE, &nd_tbl);
#ifdef CONFIG_SYSCTL
err = neigh_sysctl_register(NULL, &nd_tbl.parms,
^ permalink raw reply related
* Re: [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Joe Perches @ 2014-11-03 18:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
In-Reply-To: <1415037237.472.1.camel@linux.intel.com>
On Mon, 2014-11-03 at 19:53 +0200, Andy Shevchenko wrote:
> On Mon, 2014-11-03 at 09:39 -0800, Joe Perches wrote:
> > On Mon, 2014-11-03 at 19:28 +0200, Andy Shevchenko wrote:
> > > There is a kernel helper to dump buffers in a hexdecimal format. This patch
> > > substitutes the open coded function by calling that helper.
> > []
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > []
> > > @@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
> > >
> > > static void print_pkt(unsigned char *buf, int len)
> > > {
> > > - int j;
> > > - pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
> > > - for (j = 0; j < len; j++) {
> > > - if ((j % 16) == 0)
> > > - pr_debug("\n %03x:", j);
> > > - pr_debug(" %02x", buf[j]);
> > > - }
> > > - pr_debug("\n");
> > > + pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
> > > + print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
> >
> > Prefix should be ""
>
> Oh, initially there is an indentation in one space.
> Maybe Giuseppe would comment on this.
I think that's not particularly important.
This changes the output anyway as the the printed offset
is now 8 chars wide not 3.
> > Another (better?) option would be to use:
> >
> > print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
>
> > so that it can be dynamically controlled like
> > the pr_debug statements.
>
> Ah, yes, but unfortunately it will print ASCII part always. Giuseppe,
> what do you think?
I'm not bothered by logging message output changes.
It's not a guaranteed stable output.
> P.S. [off topic] Joe, just would like to know if you have you seen my
> last version of the patch series against hexdump.c which adds
> seq_hex_dump() call [1]. If so, could you comment on it?
> [1] https://lkml.org/lkml/2014/9/4/309
seq_<foo> output however is guaranteed.
So any conversion to the seq_hex_dump function
would need to be exactly the same.
New code could use seq_hex_dump.
Other than that, it seems sensible enough.
^ permalink raw reply
* Re: [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: Ian Campbell @ 2014-11-03 17:55 UTC (permalink / raw)
To: David Vrabel; +Cc: netdev, xen-devel, Wei Liu, Malcolm Crossley
In-Reply-To: <5457BF80.2000205@citrix.com>
On Mon, 2014-11-03 at 17:46 +0000, David Vrabel wrote:
> On 03/11/14 17:39, Ian Campbell wrote:
> > On Mon, 2014-11-03 at 17:23 +0000, David Vrabel wrote:
> >> From: Malcolm Crossley <malcolm.crossley@citrix.com>
> >>
> >> Unconditionally pulling 128 bytes into the linear buffer is not
> >> required. Netback has already grant copied up-to 128 bytes from the
> >> first slot of a packet into the linear buffer. The first slot normally
> >> contain all the IPv4/IPv6 and TCP/UDP headers.
> >
> > What about when it doesn't? It sounds as if we now won't pull up, which
> > would be bad.
>
> The network stack will always pull any headers it needs to inspect (the
> frag may be a userspace page which has the same security issues as a
> frag with a foreign page).
I don't believe it will, unless something changed since I last looked.
The kernel assumes that it has been sensible enough to put the headers
in the linear area, since it is the one which generates them in most
cases. In other cases its up to the relevant driver to make sure this is
true.
> e.g., see skb_checksum_setup() called slightly later on in netback.
This however is what will make things safe for us (note that this is
only used by xen-net* in practice), it is this which should be mentioned
in the commit message I think.
> > To avoid the pull up the code would need to grant copy up-to 128 bytes
> > from as many slots as needed, not only the first.
> >
> > Also, if the grant copy has already placed 128 bytes in the linear area,
> > why is the pull up touching anything in the first place? Shouldn't it be
> > a nop in that case?
>
> The grant copy only copies from the first frag which may be less than
> 128 bytes in length.
>
> David
^ permalink raw reply
* Re: [PATCH] PPC: bpf_jit_comp: add SKF_AD_PKTTYPE instruction
From: Denis Kirjanov @ 2014-11-03 18:04 UTC (permalink / raw)
To: Philippe Bergheaud; +Cc: linuxppc-dev, netdev, Matt Evans, mpe
In-Reply-To: <5457A306.4040302@linux.vnet.ibm.com>
On 11/3/14, Philippe Bergheaud <felix@linux.vnet.ibm.com> wrote:
> Denis Kirjanov wrote:
>> Any feedback from PPC folks?
>
> I have reviewed the patch and it looks fine to me.
> I have tested successfuly on ppc64le.
> I could not test it on ppc64.
Nice, I've tested it on PPC64be
>
> Philippe
>
>> On 10/26/14, Denis Kirjanov <kda@linux-powerpc.org> wrote:
>>
>>>Cc: Matt Evans <matt@ozlabs.org>
>>>Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>>>---
>>> arch/powerpc/include/asm/ppc-opcode.h | 1 +
>>> arch/powerpc/net/bpf_jit.h | 7 +++++++
>>> arch/powerpc/net/bpf_jit_comp.c | 5 +++++
>>> 3 files changed, 13 insertions(+)
>>>
>>>diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>>>b/arch/powerpc/include/asm/ppc-opcode.h
>>>index 6f85362..1a52877 100644
>>>--- a/arch/powerpc/include/asm/ppc-opcode.h
>>>+++ b/arch/powerpc/include/asm/ppc-opcode.h
>>>@@ -204,6 +204,7 @@
>>> #define PPC_INST_ERATSX_DOT 0x7c000127
>>>
>>> /* Misc instructions for BPF compiler */
>>>+#define PPC_INST_LBZ 0x88000000
>>> #define PPC_INST_LD 0xe8000000
>>> #define PPC_INST_LHZ 0xa0000000
>>> #define PPC_INST_LHBRX 0x7c00062c
>>>diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
>>>index 9aee27c..c406aa9 100644
>>>--- a/arch/powerpc/net/bpf_jit.h
>>>+++ b/arch/powerpc/net/bpf_jit.h
>>>@@ -87,6 +87,9 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
>>> #define PPC_STD(r, base, i) EMIT(PPC_INST_STD | ___PPC_RS(r) | \
>>> ___PPC_RA(base) | ((i) & 0xfffc))
>>>
>>>+
>>>+#define PPC_LBZ(r, base, i) EMIT(PPC_INST_LBZ | ___PPC_RT(r) | \
>>>+ ___PPC_RA(base) | IMM_L(i))
>>> #define PPC_LD(r, base, i) EMIT(PPC_INST_LD | ___PPC_RT(r) | \
>>> ___PPC_RA(base) | IMM_L(i))
>>> #define PPC_LWZ(r, base, i) EMIT(PPC_INST_LWZ | ___PPC_RT(r) | \
>>>@@ -96,6 +99,10 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
>>> #define PPC_LHBRX(r, base, b) EMIT(PPC_INST_LHBRX | ___PPC_RT(r) |
>>> \
>>> ___PPC_RA(base) | ___PPC_RB(b))
>>> /* Convenience helpers for the above with 'far' offsets: */
>>>+#define PPC_LBZ_OFFS(r, base, i) do { if ((i) < 32768) PPC_LBZ(r, base,
>>> i);
>>> \
>>>+ else { PPC_ADDIS(r, base, IMM_HA(i)); \
>>>+ PPC_LBZ(r, r, IMM_L(i)); } } while(0)
>>>+
>>> #define PPC_LD_OFFS(r, base, i) do { if ((i) < 32768) PPC_LD(r, base,
>>> i);
>>> \
>>> else { PPC_ADDIS(r, base, IMM_HA(i)); \
>>> PPC_LD(r, r, IMM_L(i)); } } while(0)
>>>diff --git a/arch/powerpc/net/bpf_jit_comp.c
>>>b/arch/powerpc/net/bpf_jit_comp.c
>>>index cbae2df..d110e28 100644
>>>--- a/arch/powerpc/net/bpf_jit_comp.c
>>>+++ b/arch/powerpc/net/bpf_jit_comp.c
>>>@@ -407,6 +407,11 @@ static int bpf_jit_build_body(struct bpf_prog *fp,
>>> u32
>>>*image,
>>> PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,
>>> queue_mapping));
>>> break;
>>>+ case BPF_ANC | SKF_AD_PKTTYPE:
>>>+ PPC_LBZ_OFFS(r_A, r_skb, PKT_TYPE_OFFSET());
>>>+ PPC_ANDI(r_A, r_A, PKT_TYPE_MAX);
>>>+ PPC_SRWI(r_A, r_A, 5);
>>>+ break;
>>> case BPF_ANC | SKF_AD_CPU:
>>> #ifdef CONFIG_SMP
>>> /*
>>>--
>>>2.1.0
>>>
>>>
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
^ permalink raw reply
* Re: [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: Ian Campbell @ 2014-11-03 17:39 UTC (permalink / raw)
To: David Vrabel; +Cc: netdev, xen-devel, Wei Liu, Malcolm Crossley
In-Reply-To: <1415035431-27485-1-git-send-email-david.vrabel@citrix.com>
On Mon, 2014-11-03 at 17:23 +0000, David Vrabel wrote:
> From: Malcolm Crossley <malcolm.crossley@citrix.com>
>
> Unconditionally pulling 128 bytes into the linear buffer is not
> required. Netback has already grant copied up-to 128 bytes from the
> first slot of a packet into the linear buffer. The first slot normally
> contain all the IPv4/IPv6 and TCP/UDP headers.
What about when it doesn't? It sounds as if we now won't pull up, which
would be bad.
To avoid the pull up the code would need to grant copy up-to 128 bytes
from as many slots as needed, not only the first.
Also, if the grant copy has already placed 128 bytes in the linear area,
why is the pull up touching anything in the first place? Shouldn't it be
a nop in that case?
> The unconditional pull would often copy frag data unnecessarily. This
> is a performance problem when running on a version of Xen where grant
> unmap avoids TLB flushes for pages which are not accessed. TLB
> flushes can now be avoided for > 99% of unmaps (it was 0% before).
>
> Grant unmap TLB flush avoidance will be available in a future version
> of Xen (probably 4.6).
>
> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> drivers/net/xen-netback/netback.c | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 730252c..14e18bb 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -82,6 +82,16 @@ MODULE_PARM_DESC(max_queues,
> static unsigned int fatal_skb_slots = FATAL_SKB_SLOTS_DEFAULT;
> module_param(fatal_skb_slots, uint, 0444);
>
> +/* The amount to copy out of the first guest Tx slot into the skb's
> + * linear area. If the first slot has more data, it will be mapped
> + * and put into the first frag.
> + *
> + * This is sized to avoid pulling headers from the frags for most
> + * TCP/IP packets.
> + */
> +#define XEN_NETBACK_TX_COPY_LEN 128
Was it strictly necessary to both move and rename this? I can see why
the comment needed to change.
> static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
> u8 status);
>
> @@ -125,13 +135,6 @@ static inline struct xenvif_queue *ubuf_to_queue(const struct ubuf_info *ubuf)
> pending_tx_info[0]);
> }
>
> -/* This is a miniumum size for the linear area to avoid lots of
> - * calls to __pskb_pull_tail() as we set up checksum offsets. The
> - * value 128 was chosen as it covers all IPv4 and most likely
> - * IPv6 headers.
> - */
> -#define PKT_PROT_LEN 128
> -
> static u16 frag_get_pending_idx(skb_frag_t *frag)
> {
> return (u16)frag->page_offset;
> @@ -1446,9 +1449,9 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
> index = pending_index(queue->pending_cons);
> pending_idx = queue->pending_ring[index];
>
> - data_len = (txreq.size > PKT_PROT_LEN &&
> + data_len = (txreq.size > XEN_NETBACK_TX_COPY_LEN &&
> ret < XEN_NETBK_LEGACY_SLOTS_MAX) ?
> - PKT_PROT_LEN : txreq.size;
> + XEN_NETBACK_TX_COPY_LEN : txreq.size;
>
> skb = xenvif_alloc_skb(data_len);
> if (unlikely(skb == NULL)) {
> @@ -1653,11 +1656,6 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
> }
> }
>
> - if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
> - int target = min_t(int, skb->len, PKT_PROT_LEN);
> - __pskb_pull_tail(skb, target - skb_headlen(skb));
> - }
> -
> skb->dev = queue->vif->dev;
> skb->protocol = eth_type_trans(skb, skb->dev);
> skb_reset_network_header(skb);
^ permalink raw reply
* Re: [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Andy Shevchenko @ 2014-11-03 17:53 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
In-Reply-To: <1415036355.17743.22.camel@perches.com>
On Mon, 2014-11-03 at 09:39 -0800, Joe Perches wrote:
> On Mon, 2014-11-03 at 19:28 +0200, Andy Shevchenko wrote:
> > There is a kernel helper to dump buffers in a hexdecimal format. This patch
> > substitutes the open coded function by calling that helper.
> []
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> []
> > @@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
> >
> > static void print_pkt(unsigned char *buf, int len)
> > {
> > - int j;
> > - pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
> > - for (j = 0; j < len; j++) {
> > - if ((j % 16) == 0)
> > - pr_debug("\n %03x:", j);
> > - pr_debug(" %02x", buf[j]);
> > - }
> > - pr_debug("\n");
> > + pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
> > + print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
>
> Prefix should be ""
Oh, initially there is an indentation in one space.
Maybe Giuseppe would comment on this.
> Please use true/false for the last argument
Okay.
>
> Another (better?) option would be to use:
>
> print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
> so that it can be dynamically controlled like
> the pr_debug statements.
Ah, yes, but unfortunately it will print ASCII part always. Giuseppe,
what do you think?
P.S. [off topic] Joe, just would like to know if you have you seen my
last version of the patch series against hexdump.c which adds
seq_hex_dump() call [1]. If so, could you comment on it?
[1] https://lkml.org/lkml/2014/9/4/309
--
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: David Vrabel @ 2014-11-03 17:46 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, xen-devel, Wei Liu, Malcolm Crossley
In-Reply-To: <1415036346.1411.3.camel@citrix.com>
On 03/11/14 17:39, Ian Campbell wrote:
> On Mon, 2014-11-03 at 17:23 +0000, David Vrabel wrote:
>> From: Malcolm Crossley <malcolm.crossley@citrix.com>
>>
>> Unconditionally pulling 128 bytes into the linear buffer is not
>> required. Netback has already grant copied up-to 128 bytes from the
>> first slot of a packet into the linear buffer. The first slot normally
>> contain all the IPv4/IPv6 and TCP/UDP headers.
>
> What about when it doesn't? It sounds as if we now won't pull up, which
> would be bad.
The network stack will always pull any headers it needs to inspect (the
frag may be a userspace page which has the same security issues as a
frag with a foreign page).
e.g., see skb_checksum_setup() called slightly later on in netback.
> To avoid the pull up the code would need to grant copy up-to 128 bytes
> from as many slots as needed, not only the first.
>
> Also, if the grant copy has already placed 128 bytes in the linear area,
> why is the pull up touching anything in the first place? Shouldn't it be
> a nop in that case?
The grant copy only copies from the first frag which may be less than
128 bytes in length.
David
^ permalink raw reply
* Re: [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Joe Perches @ 2014-11-03 17:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
In-Reply-To: <1415035734-24163-2-git-send-email-andriy.shevchenko@linux.intel.com>
On Mon, 2014-11-03 at 19:28 +0200, Andy Shevchenko wrote:
> There is a kernel helper to dump buffers in a hexdecimal format. This patch
> substitutes the open coded function by calling that helper.
[]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
[]
> @@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
>
> static void print_pkt(unsigned char *buf, int len)
> {
> - int j;
> - pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
> - for (j = 0; j < len; j++) {
> - if ((j % 16) == 0)
> - pr_debug("\n %03x:", j);
> - pr_debug(" %02x", buf[j]);
> - }
> - pr_debug("\n");
> + pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
> + print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
Prefix should be ""
Please use true/false for the last argument
Another (better?) option would be to use:
print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
so that it can be dynamically controlled like
the pr_debug statements.
^ permalink raw reply
* Re: [PATCH net] uapi: add missing network related headers to kbuild
From: David Miller @ 2014-11-03 17:33 UTC (permalink / raw)
To: stephen; +Cc: netdev
In-Reply-To: <20141102113141.7aa1065c@urahara>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 2 Nov 2014 11:31:41 -0800
> The makefile for sanitizing kernel headers uses the kbuild file
> to determine which files to do. Several networking related headers
> were missing. Without these headers iproute2 build would break.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied, thanks Stephen.
^ permalink raw reply
* [PATCH] stmmac: fix sparse warnings
From: Andy Shevchenko @ 2014-11-03 17:28 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
Cc: Andy Shevchenko
This patch fixes the following sparse warnings.
drivers/net/ethernet/stmicro/stmmac/enh_desc.c:381:30: warning: symbol 'enh_desc_ops' was not declared. Should it be static?
drivers/net/ethernet/stmicro/stmmac/norm_desc.c:253:30: warning: symbol 'ndesc_ops' was not declared. Should it be static?
drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c:141:33: warning: symbol 'stmmac_ptp' was not declared. Should it be static?
There is no functional change.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index 1e2bcf5..ddd4272 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -23,8 +23,10 @@
*******************************************************************************/
#include <linux/stmmac.h>
+
#include "common.h"
#include "descs_com.h"
+#include "stmmac.h"
static int enh_desc_get_tx_status(void *data, struct stmmac_extra_stats *x,
struct dma_desc *p, void __iomem *ioaddr)
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index 35ad4f4..46b882c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -23,8 +23,10 @@
*******************************************************************************/
#include <linux/stmmac.h>
+
#include "common.h"
#include "descs_com.h"
+#include "stmmac.h"
static int ndesc_get_tx_status(void *data, struct stmmac_extra_stats *x,
struct dma_desc *p, void __iomem *ioaddr)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
index 76ad214..88e0da3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
@@ -25,8 +25,10 @@
#include <linux/io.h>
#include <linux/delay.h>
+
#include "common.h"
#include "stmmac_ptp.h"
+#include "stmmac.h"
static void stmmac_config_hw_tstamping(void __iomem *ioaddr, u32 data)
{
--
2.1.1
^ permalink raw reply related
* Re: Connection with Smart Z hangs
From: David Miller @ 2014-11-03 17:32 UTC (permalink / raw)
To: torvalds; +Cc: mlg.hessigheim, devel, netdev
In-Reply-To: <CA+55aFzB+d8+pjvuPo-n5G7X9MK9vh9ER5zYLCXZBzjMD4bseA@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 2 Nov 2014 10:20:22 -0800
> David, I'm just going to remove that whole bogus disconnect call. It
> won't make things *work* for Martin (because this is all in the
> "connect failed" path), but that call as-is is just wrong, wrong,
> wrong. And apparently nobody cares about irda any more.
I totally agree, just kill it. For the record this got added in:
commit 5b40964eadea40509d353318d2c82e8b7bf5e8a5
Author: Samuel Ortiz <samuel@sortiz.org>
Date: Mon Oct 11 00:46:51 2010 +0200
irda: Remove BKL instances from af_irda.c
which claims to be just a locking change, but obviously added
this bogus ->disconnect() call as well.
> If anybody is at all interested in helping maintain irda code, holler
> to David and to the netdev mailing list. The position is up for grabs.
Seriously... :-)
^ permalink raw reply
* [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Andy Shevchenko @ 2014-11-03 17:28 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
Cc: Andy Shevchenko
In-Reply-To: <1415035734-24163-1-git-send-email-andriy.shevchenko@linux.intel.com>
There is a kernel helper to dump buffers in a hexdecimal format. This patch
substitutes the open coded function by calling that helper.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6f77a46..5fb87f5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
static void print_pkt(unsigned char *buf, int len)
{
- int j;
- pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
- for (j = 0; j < len; j++) {
- if ((j % 16) == 0)
- pr_debug("\n %03x:", j);
- pr_debug(" %02x", buf[j]);
- }
- pr_debug("\n");
+ pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
+ print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
}
/* minimum number of free TX descriptors required to wake up TX process */
--
2.1.1
^ permalink raw reply related
* Re: [PATCH V1 net-next 0/5] Mellanox ethernet driver update Oct-30-2014
From: David Miller @ 2014-11-03 17:28 UTC (permalink / raw)
To: ogerlitz; +Cc: netdev, matanb, amirv, saeedm, shanim, idos
In-Reply-To: <1414938377-421-1-git-send-email-ogerlitz@mellanox.com>
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Sun, 2 Nov 2014 16:26:12 +0200
> The 1st patch from Saeed fixes a bug in the last net-next batch where
> a VF could get access to set port configuration, the next patch from Amir
> fixes a race in the port VPI logic. Next are two performance patches from Ido.
>
> The patch to add checksum complete status on GRE and such packets was
> preceded with a patch that converted the driver to only use napi_gro_receive
> vs. the current code which goes through napi_gro_frags on it's usual track.
> Eric D. has some thoughts and suggestions on that change for which we
> want to take the time and consider, so for the time being dropped that
> patch and the ones that depend on it.
...
> Changes from V0:
> - have the caller to provide the __GFP_COLD hint to the service function
> - dropped the patch that changes the GRO logic and the subsequent dependent
> patches.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: less interrupt masking in NAPI
From: David Miller @ 2014-11-03 17:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, willemb
In-Reply-To: <1414937973.31792.37.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 02 Nov 2014 06:19:33 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> net_rx_action() can mask irqs a single time to transfert sd->poll_list
> into a private list, for a very short duration.
>
> Then, napi_complete() can avoid masking irqs again,
> and net_rx_action() only needs to mask irq again in slow path.
>
> This patch removes 2 couples of irq mask/unmask per typical NAPI run,
> more if multiple napi were triggered.
>
> Note this also allows to give control back to caller (do_softirq())
> more often, so that other softirq handlers can be called a bit earlier,
> or ksoftirqd can be wakeup earlier under pressure.
>
> This was developed while testing an alternative to RX interrupt
> mitigation to reduce latencies while keeping or improving GRO
> aggregation on fast NIC.
>
> Idea is to test napi->gro_list at the end of a napi->poll() and
> reschedule one NAPI poll, but after servicing a full round of
> softirqs (timers, TX, rcu, ...). This will be allowed only if softirq
> is currently serviced by idle task or ksoftirqd, and resched not needed.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Also applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] net: shrink struct softnet_data
From: David Miller @ 2014-11-03 17:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1414936812.31792.31.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 02 Nov 2014 06:00:12 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> flow_limit in struct softnet_data is only read from local cpu
> and can be moved to fill a hole, reducing softnet_data size by
> 64 bytes on x86_64
>
> While we are at it, move output_queue, output_queue_tailp and
> completion_queue, so that rx / tx paths touch a single cache line.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: David Vrabel @ 2014-11-03 17:23 UTC (permalink / raw)
To: netdev; +Cc: David Vrabel, xen-devel, Ian Campbell, Wei Liu, Malcolm Crossley
From: Malcolm Crossley <malcolm.crossley@citrix.com>
Unconditionally pulling 128 bytes into the linear buffer is not
required. Netback has already grant copied up-to 128 bytes from the
first slot of a packet into the linear buffer. The first slot normally
contain all the IPv4/IPv6 and TCP/UDP headers.
The unconditional pull would often copy frag data unnecessarily. This
is a performance problem when running on a version of Xen where grant
unmap avoids TLB flushes for pages which are not accessed. TLB
flushes can now be avoided for > 99% of unmaps (it was 0% before).
Grant unmap TLB flush avoidance will be available in a future version
of Xen (probably 4.6).
Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
drivers/net/xen-netback/netback.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 730252c..14e18bb 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -82,6 +82,16 @@ MODULE_PARM_DESC(max_queues,
static unsigned int fatal_skb_slots = FATAL_SKB_SLOTS_DEFAULT;
module_param(fatal_skb_slots, uint, 0444);
+/* The amount to copy out of the first guest Tx slot into the skb's
+ * linear area. If the first slot has more data, it will be mapped
+ * and put into the first frag.
+ *
+ * This is sized to avoid pulling headers from the frags for most
+ * TCP/IP packets.
+ */
+#define XEN_NETBACK_TX_COPY_LEN 128
+
+
static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
u8 status);
@@ -125,13 +135,6 @@ static inline struct xenvif_queue *ubuf_to_queue(const struct ubuf_info *ubuf)
pending_tx_info[0]);
}
-/* This is a miniumum size for the linear area to avoid lots of
- * calls to __pskb_pull_tail() as we set up checksum offsets. The
- * value 128 was chosen as it covers all IPv4 and most likely
- * IPv6 headers.
- */
-#define PKT_PROT_LEN 128
-
static u16 frag_get_pending_idx(skb_frag_t *frag)
{
return (u16)frag->page_offset;
@@ -1446,9 +1449,9 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
index = pending_index(queue->pending_cons);
pending_idx = queue->pending_ring[index];
- data_len = (txreq.size > PKT_PROT_LEN &&
+ data_len = (txreq.size > XEN_NETBACK_TX_COPY_LEN &&
ret < XEN_NETBK_LEGACY_SLOTS_MAX) ?
- PKT_PROT_LEN : txreq.size;
+ XEN_NETBACK_TX_COPY_LEN : txreq.size;
skb = xenvif_alloc_skb(data_len);
if (unlikely(skb == NULL)) {
@@ -1653,11 +1656,6 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
}
}
- if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
- int target = min_t(int, skb->len, PKT_PROT_LEN);
- __pskb_pull_tail(skb, target - skb_headlen(skb));
- }
-
skb->dev = queue->vif->dev;
skb->protocol = eth_type_trans(skb, skb->dev);
skb_reset_network_header(skb);
--
1.7.10.4
^ permalink raw reply related
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