* [PATCHv2 net-next 4/7] smsc911x: replace printk with netdev_ calls
From: Ben Boeckel @ 2013-11-01 4:27 UTC (permalink / raw)
To: netdev; +Cc: Ben Boeckel
In-Reply-To: <1383280043-13552-1-git-send-email-mathstuf@gmail.com>
Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
drivers/net/ethernet/smsc/smsc911x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 01f8459..8564f23 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2167,7 +2167,7 @@ static int smsc911x_init(struct net_device *dev)
udelay(1000);
if (to == 0) {
- pr_err("Device not READY in 100ms aborting\n");
+ netdev_err(dev, "Device not READY in 100ms aborting\n");
return -ENODEV;
}
--
1.8.4.2
^ permalink raw reply related
* [PATCHv2 net-next 5/7] smc9194: replace printk with netdev_ calls
From: Ben Boeckel @ 2013-11-01 4:27 UTC (permalink / raw)
To: netdev; +Cc: Ben Boeckel
In-Reply-To: <1383280043-13552-1-git-send-email-mathstuf@gmail.com>
Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
drivers/net/ethernet/smsc/smc9194.c | 52 ++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smc9194.c b/drivers/net/ethernet/smsc/smc9194.c
index e85c2e7..51aa9cf 100644
--- a/drivers/net/ethernet/smsc/smc9194.c
+++ b/drivers/net/ethernet/smsc/smc9194.c
@@ -55,7 +55,7 @@
----------------------------------------------------------------------------*/
static const char version[] =
- "smc9194.c:v0.14 12/15/00 by Erik Stahlman (erik@vt.edu)\n";
+ "smc9194.c:v0.14 12/15/00 by Erik Stahlman (erik@vt.edu)";
#include <linux/module.h>
#include <linux/kernel.h>
@@ -612,7 +612,7 @@ static void smc_hardware_send_packet( struct net_device * dev )
packet_no = inb( ioaddr + PNR_ARR + 1 );
if ( packet_no & 0x80 ) {
/* or isn't there? BAD CHIP! */
- printk(KERN_DEBUG CARDNAME": Memory allocation failed.\n");
+ netdev_dbg(dev, CARDNAME": Memory allocation failed.\n");
dev_kfree_skb_any(skb);
lp->saved_skb = NULL;
netif_wake_queue(dev);
@@ -625,7 +625,7 @@ static void smc_hardware_send_packet( struct net_device * dev )
/* point to the beginning of the packet */
outw( PTR_AUTOINC , ioaddr + POINTER );
- PRINTK3((CARDNAME": Trying to xmit packet of length %x\n", length ));
+ PRINTK3((CARDNAME": Trying to xmit packet of length %x\n", length));
#if SMC_DEBUG > 2
print_packet( buf, length );
#endif
@@ -865,7 +865,6 @@ static const struct net_device_ops smc_netdev_ops = {
static int __init smc_probe(struct net_device *dev, int ioaddr)
{
int i, memory, retval;
- static unsigned version_printed;
unsigned int bank;
const char *version_string;
@@ -937,8 +936,7 @@ static int __init smc_probe(struct net_device *dev, int ioaddr)
It might be prudent to check a listing of MAC addresses
against the hardware address, or do some other tests. */
- if (version_printed++ == 0)
- printk("%s", version);
+ pr_info_once("%s\n", version);
/* fill in some of the fields */
dev->base_addr = ioaddr;
@@ -1027,21 +1025,21 @@ static int __init smc_probe(struct net_device *dev, int ioaddr)
/* now, print out the card info, in a short format.. */
- printk("%s: %s(r:%d) at %#3x IRQ:%d INTF:%s MEM:%db ", dev->name,
- version_string, revision_register & 0xF, ioaddr, dev->irq,
- if_string, memory );
+ netdev_info(dev, "%s(r:%d) at %#3x IRQ:%d INTF:%s MEM:%db ",
+ version_string, revision_register & 0xF, ioaddr, dev->irq,
+ if_string, memory);
/*
. Print the Ethernet address
*/
- printk("ADDR: %pM\n", dev->dev_addr);
+ netdev_info(dev, "ADDR: %pM\n", dev->dev_addr);
/* Grab the IRQ */
- retval = request_irq(dev->irq, smc_interrupt, 0, DRV_NAME, dev);
- if (retval) {
- printk("%s: unable to get IRQ %d (irqval=%d).\n", DRV_NAME,
- dev->irq, retval);
- goto err_out;
- }
+ retval = request_irq(dev->irq, smc_interrupt, 0, DRV_NAME, dev);
+ if (retval) {
+ netdev_warn(dev, "%s: unable to get IRQ %d (irqval=%d).\n",
+ DRV_NAME, dev->irq, retval);
+ goto err_out;
+ }
dev->netdev_ops = &smc_netdev_ops;
dev->watchdog_timeo = HZ/20;
@@ -1061,30 +1059,32 @@ static void print_packet( byte * buf, int length )
int remainder;
int lines;
- printk("Packet of length %d\n", length);
+ pr_dbg("Packet of length %d\n", length);
lines = length / 16;
remainder = length % 16;
for ( i = 0; i < lines ; i ++ ) {
int cur;
+ printk(KERN_DEBUG);
for ( cur = 0; cur < 8; cur ++ ) {
byte a, b;
a = *(buf ++ );
b = *(buf ++ );
- printk("%02x%02x ", a, b );
+ pr_cont("%02x%02x ", a, b);
}
- printk("\n");
+ pr_cont("\n");
}
+ printk(KERN_DEBUG);
for ( i = 0; i < remainder/2 ; i++ ) {
byte a, b;
a = *(buf ++ );
b = *(buf ++ );
- printk("%02x%02x ", a, b );
+ pr_cont("%02x%02x ", a, b);
}
- printk("\n");
+ pr_cont("\n");
#endif
}
#endif
@@ -1151,9 +1151,8 @@ static void smc_timeout(struct net_device *dev)
{
/* If we get here, some higher level has decided we are broken.
There should really be a "kick me" function call instead. */
- printk(KERN_WARNING CARDNAME": transmit timed out, %s?\n",
- tx_done(dev) ? "IRQ conflict" :
- "network cable problem");
+ netdev_warn(dev, CARDNAME": transmit timed out, %s?\n",
+ tx_done(dev) ? "IRQ conflict" : "network cable problem");
/* "kick" the adaptor */
smc_reset( dev->base_addr );
smc_enable( dev->base_addr );
@@ -1323,8 +1322,7 @@ static void smc_tx( struct net_device * dev )
dev->stats.tx_errors++;
if ( tx_status & TS_LOSTCAR ) dev->stats.tx_carrier_errors++;
if ( tx_status & TS_LATCOL ) {
- printk(KERN_DEBUG CARDNAME
- ": Late collision occurred on last xmit.\n");
+ netdev_dbg(dev, CARDNAME": Late collision occurred on last xmit.\n");
dev->stats.tx_window_errors++;
}
#if 0
@@ -1332,7 +1330,7 @@ static void smc_tx( struct net_device * dev )
#endif
if ( tx_status & TS_SUCCESS ) {
- printk(CARDNAME": Successful packet caused interrupt\n");
+ netdev_info(dev, CARDNAME": Successful packet caused interrupt\n");
}
/* re-enable transmit */
SMC_SELECT_BANK( 0 );
--
1.8.4.2
^ permalink raw reply related
* [PATCHv2 net-next 6/7] smc91c92_cs: replace printk with netdev_ calls
From: Ben Boeckel @ 2013-11-01 4:27 UTC (permalink / raw)
To: netdev; +Cc: Ben Boeckel
In-Reply-To: <1383280043-13552-1-git-send-email-mathstuf@gmail.com>
Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
drivers/net/ethernet/smsc/smc91c92_cs.c | 43 ++++++++++++++++-----------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smc91c92_cs.c b/drivers/net/ethernet/smsc/smc91c92_cs.c
index 656d2e2..8ef70d9 100644
--- a/drivers/net/ethernet/smsc/smc91c92_cs.c
+++ b/drivers/net/ethernet/smsc/smc91c92_cs.c
@@ -740,7 +740,7 @@ static int smc91c92_resume(struct pcmcia_device *link)
(smc->cardid == PRODID_PSION_NET100))) {
i = osi_load_firmware(link);
if (i) {
- pr_err("smc91c92_cs: Failed to load firmware\n");
+ netdev_err(dev, "Failed to load firmware\n");
return i;
}
}
@@ -793,7 +793,7 @@ static int check_sig(struct pcmcia_device *link)
}
if (width) {
- pr_info("using 8-bit IO window\n");
+ netdev_info(dev, "using 8-bit IO window\n");
smc91c92_suspend(link);
pcmcia_fixup_iowidth(link);
@@ -1036,7 +1036,7 @@ static void smc_dump(struct net_device *dev)
save = inw(ioaddr + BANK_SELECT);
for (w = 0; w < 4; w++) {
SMC_SELECT_BANK(w);
- netdev_printk(KERN_DEBUG, dev, "bank %d: ", w);
+ netdev_dbg(dev, "bank %d: ", w);
for (i = 0; i < 14; i += 2)
pr_cont(" %04x", inw(ioaddr + i));
pr_cont("\n");
@@ -1213,8 +1213,7 @@ static netdev_tx_t smc_start_xmit(struct sk_buff *skb,
if (smc->saved_skb) {
/* THIS SHOULD NEVER HAPPEN. */
dev->stats.tx_aborted_errors++;
- netdev_printk(KERN_DEBUG, dev,
- "Internal error -- sent packet while busy\n");
+ netdev_dbg(dev, "Internal error -- sent packet while busy\n");
return NETDEV_TX_BUSY;
}
smc->saved_skb = skb;
@@ -1254,7 +1253,7 @@ static netdev_tx_t smc_start_xmit(struct sk_buff *skb,
}
/* Otherwise defer until the Tx-space-allocated interrupt. */
- pr_debug("%s: memory allocation deferred.\n", dev->name);
+ netdev_dbg(dev, "memory allocation deferred.\n");
outw((IM_ALLOC_INT << 8) | (ir & 0xff00), ioaddr + INTERRUPT);
spin_unlock_irqrestore(&smc->lock, flags);
@@ -1317,8 +1316,8 @@ static void smc_eph_irq(struct net_device *dev)
SMC_SELECT_BANK(0);
ephs = inw(ioaddr + EPH);
- pr_debug("%s: Ethernet protocol handler interrupt, status"
- " %4.4x.\n", dev->name, ephs);
+ netdev_dbg(dev, "Ethernet protocol handler interrupt, status %4.4x.\n",
+ ephs);
/* Could be a counter roll-over warning: update stats. */
card_stats = inw(ioaddr + COUNTER);
/* single collisions */
@@ -1357,8 +1356,8 @@ static irqreturn_t smc_interrupt(int irq, void *dev_id)
ioaddr = dev->base_addr;
- pr_debug("%s: SMC91c92 interrupt %d at %#x.\n", dev->name,
- irq, ioaddr);
+ netdev_dbg(dev, "SMC91c92 interrupt %d at %#x.\n",
+ irq, ioaddr);
spin_lock(&smc->lock);
smc->watchdog = 0;
@@ -1366,8 +1365,8 @@ static irqreturn_t smc_interrupt(int irq, void *dev_id)
if ((saved_bank & 0xff00) != 0x3300) {
/* The device does not exist -- the card could be off-line, or
maybe it has been ejected. */
- pr_debug("%s: SMC91c92 interrupt %d for non-existent"
- "/ejected device.\n", dev->name, irq);
+ netdev_dbg(dev, "SMC91c92 interrupt %d for non-existent/ejected device.\n",
+ irq);
handled = 0;
goto irq_done;
}
@@ -1380,8 +1379,8 @@ static irqreturn_t smc_interrupt(int irq, void *dev_id)
do { /* read the status flag, and mask it */
status = inw(ioaddr + INTERRUPT) & 0xff;
- pr_debug("%s: Status is %#2.2x (mask %#2.2x).\n", dev->name,
- status, mask);
+ netdev_dbg(dev, "Status is %#2.2x (mask %#2.2x).\n",
+ status, mask);
if ((status & mask) == 0) {
if (bogus_cnt == INTR_WORK)
handled = 0;
@@ -1425,15 +1424,15 @@ static irqreturn_t smc_interrupt(int irq, void *dev_id)
smc_eph_irq(dev);
} while (--bogus_cnt);
- pr_debug(" Restoring saved registers mask %2.2x bank %4.4x"
- " pointer %4.4x.\n", mask, saved_bank, saved_pointer);
+ netdev_dbg(dev, " Restoring saved registers mask %2.2x bank %4.4x pointer %4.4x.\n",
+ mask, saved_bank, saved_pointer);
/* restore state register */
outw((mask<<8), ioaddr + INTERRUPT);
outw(saved_pointer, ioaddr + POINTER);
SMC_SELECT_BANK(saved_bank);
- pr_debug("%s: Exiting interrupt IRQ%d.\n", dev->name, irq);
+ netdev_dbg(dev, "Exiting interrupt IRQ%d.\n", irq);
irq_done:
@@ -1491,10 +1490,10 @@ static void smc_rx(struct net_device *dev)
rx_status = inw(ioaddr + DATA_1);
packet_length = inw(ioaddr + DATA_1) & 0x07ff;
- pr_debug("%s: Receive status %4.4x length %d.\n",
- dev->name, rx_status, packet_length);
+ netdev_dbg(dev, "Receive status %4.4x length %d.\n",
+ rx_status, packet_length);
- if (!(rx_status & RS_ERRORS)) {
+ if (!(rx_status & RS_ERRORS)) {
/* do stuff to make a new packet */
struct sk_buff *skb;
@@ -1502,7 +1501,7 @@ static void smc_rx(struct net_device *dev)
skb = netdev_alloc_skb(dev, packet_length+2);
if (skb == NULL) {
- pr_debug("%s: Low memory, packet dropped.\n", dev->name);
+ netdev_dbg(dev, "Low memory, packet dropped.\n");
dev->stats.rx_dropped++;
outw(MC_RELEASE, ioaddr + MMU_CMD);
return;
@@ -1643,7 +1642,7 @@ static void smc_reset(struct net_device *dev)
struct smc_private *smc = netdev_priv(dev);
int i;
- pr_debug("%s: smc91c92 reset called.\n", dev->name);
+ netdev_dbg(dev, "smc91c92 reset called.\n");
/* The first interaction must be a write to bring the chip out
of sleep mode. */
--
1.8.4.2
^ permalink raw reply related
* [PATCHv2 net-next 7/7] smsc9420: replace printk with netdev_ calls
From: Ben Boeckel @ 2013-11-01 4:27 UTC (permalink / raw)
To: netdev; +Cc: Ben Boeckel
In-Reply-To: <1383280043-13552-1-git-send-email-mathstuf@gmail.com>
Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
drivers/net/ethernet/smsc/smsc9420.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smsc9420.c b/drivers/net/ethernet/smsc/smsc9420.c
index e55e336..059bcaf 100644
--- a/drivers/net/ethernet/smsc/smsc9420.c
+++ b/drivers/net/ethernet/smsc/smsc9420.c
@@ -99,17 +99,17 @@ MODULE_PARM_DESC(debug, "debug level");
#define smsc_dbg(TYPE, f, a...) \
do { if ((pd)->msg_enable & NETIF_MSG_##TYPE) \
- printk(KERN_DEBUG PFX f "\n", ## a); \
+ netdev_dbg((pd)->dev, PFX f "\n", ## a); \
} while (0)
#define smsc_info(TYPE, f, a...) \
do { if ((pd)->msg_enable & NETIF_MSG_##TYPE) \
- printk(KERN_INFO PFX f "\n", ## a); \
+ netdev_info((pd)->dev, PFX f "\n", ## a); \
} while (0)
#define smsc_warn(TYPE, f, a...) \
do { if ((pd)->msg_enable & NETIF_MSG_##TYPE) \
- printk(KERN_WARNING PFX f "\n", ## a); \
+ netdev_warn((pd)->dev, PFX f "\n", ## a); \
} while (0)
static inline u32 smsc9420_reg_read(struct smsc9420_pdata *pd, u32 offset)
@@ -1168,7 +1168,7 @@ static int smsc9420_mii_probe(struct net_device *dev)
/* Device only supports internal PHY at address 1 */
if (!pd->mii_bus->phy_map[1]) {
- pr_err("%s: no PHY found at address 1\n", dev->name);
+ netdev_err(dev, "no PHY found at address 1\n");
return -ENODEV;
}
@@ -1180,12 +1180,12 @@ static int smsc9420_mii_probe(struct net_device *dev)
smsc9420_phy_adjust_link, PHY_INTERFACE_MODE_MII);
if (IS_ERR(phydev)) {
- pr_err("%s: Could not attach to PHY\n", dev->name);
+ netdev_err(dev, "Could not attach to PHY\n");
return PTR_ERR(phydev);
}
- pr_info("%s: attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
- dev->name, phydev->drv->name, dev_name(&phydev->dev), phydev->irq);
+ netdev_info(dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
+ phydev->drv->name, dev_name(&phydev->dev), phydev->irq);
/* mask with MAC supported features */
phydev->supported &= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
@@ -1582,12 +1582,12 @@ smsc9420_probe(struct pci_dev *pdev, const struct pci_device_id *id)
int result = 0;
u32 id_rev;
- printk(KERN_INFO DRV_DESCRIPTION " version " DRV_VERSION "\n");
+ pr_info(DRV_DESCRIPTION " version " DRV_VERSION "\n");
/* First do the PCI initialisation */
result = pci_enable_device(pdev);
if (unlikely(result)) {
- printk(KERN_ERR "Cannot enable smsc9420\n");
+ pr_err("Cannot enable smsc9420\n");
goto out_0;
}
@@ -1600,24 +1600,24 @@ smsc9420_probe(struct pci_dev *pdev, const struct pci_device_id *id)
SET_NETDEV_DEV(dev, &pdev->dev);
if (!(pci_resource_flags(pdev, SMSC_BAR) & IORESOURCE_MEM)) {
- printk(KERN_ERR "Cannot find PCI device base address\n");
+ netdev_err(dev, "Cannot find PCI device base address\n");
goto out_free_netdev_2;
}
if ((pci_request_regions(pdev, DRV_NAME))) {
- printk(KERN_ERR "Cannot obtain PCI resources, aborting.\n");
+ netdev_err(dev, "Cannot obtain PCI resources, aborting.\n");
goto out_free_netdev_2;
}
if (pci_set_dma_mask(pdev, DMA_BIT_MASK(32))) {
- printk(KERN_ERR "No usable DMA configuration, aborting.\n");
+ netdev_err(dev, "No usable DMA configuration, aborting.\n");
goto out_free_regions_3;
}
virt_addr = ioremap(pci_resource_start(pdev, SMSC_BAR),
pci_resource_len(pdev, SMSC_BAR));
if (!virt_addr) {
- printk(KERN_ERR "Cannot map device registers, aborting.\n");
+ netdev_err(dev, "Cannot map device registers, aborting.\n");
goto out_free_regions_3;
}
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH net-next 1/6] epic100: replace printk with netdev_ calls
From: Ben Boeckel @ 2013-11-01 4:31 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev
In-Reply-To: <1382683266.2068.13.camel@joe-AO722>
On Thu, Oct 24, 2013 at 23:41:06 -0700, Joe Perches wrote:
> On Fri, 2013-10-25 at 02:26 -0400, Ben Boeckel wrote:
> Please run your patches through checkpatch.pl
Done. There are some 80 character limit issues yet, but they're literal
strings for the most part. I also left rampant space-for-indent as-is
(though I did nab a few space-between-tab instances I spotted).
> > diff --git a/drivers/net/ethernet/smsc/epic100.c b/drivers/net/ethernet/smsc/epic100.c
> []
> > @@ -490,7 +488,7 @@ static int epic_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> > if (ret < 0)
> > goto err_out_unmap_rx;
> >
> > - printk(KERN_INFO "%s: %s at %lx, IRQ %d, %pM\n",
> > + netdev_info(dev, "%s: %s at %lx, IRQ %d, %pM\n",
> > dev->name, pci_id_tbl[chip_idx].name,
> > (long)pci_resource_start(pdev, EPIC_BAR), pdev->irq,
> > dev->dev_addr);
>
> Please align the subsequent lines to the open parenthesis
> and this conversion would now emit dev->name twice.
Done.
> Please coalesce the format fragments into a single line.
>
> > dev->name, mdio_read(dev, ep->phys[0], MII_BMSR));
>
> same problem with duplicate dev->name
Done.
--Ben
^ permalink raw reply
* Re: [PATCHv2 net-next 1/7] epic100: replace printk with netdev_ calls
From: Joe Perches @ 2013-11-01 4:36 UTC (permalink / raw)
To: Ben Boeckel; +Cc: netdev
In-Reply-To: <1383280043-13552-2-git-send-email-mathstuf@gmail.com>
On Fri, 2013-11-01 at 00:27 -0400, Ben Boeckel wrote:
> Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
Nice (really, nicely done!) but:
> diff --git a/drivers/net/ethernet/smsc/epic100.c b/drivers/net/ethernet/smsc/epic100.c
[]
> @@ -1324,7 +1316,7 @@ static int epic_close(struct net_device *dev)
> ep->rx_ring[i].buflength = 0;
> if (skb) {
> pci_unmap_single(pdev, ep->rx_ring[i].bufaddr,
> - ep->rx_buf_sz, PCI_DMA_FROMDEVICE);
> + ep->rx_buf_sz, PCI_DMA_FROMDEVICE);
Please either mention the whitespace changes in the
changelog or don't do them in the same patch.
^ permalink raw reply
* [PATCH net-next] vxlan: Use ERR_CAST inlined function instead of ERR_PTR(PTR_ERR(...))
From: Duan Jiong @ 2013-11-01 5:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev
trivial patch converting ERR_PTR(PTR_ERR()) into ERR_CAST().
No functional changes.
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
drivers/net/vxlan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 24260ce..78df8f3 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2285,7 +2285,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
sock = create_v4_sock(net, port);
if (IS_ERR(sock)) {
kfree(vs);
- return ERR_PTR(PTR_ERR(sock));
+ return ERR_CAST(sock);
}
vs->sock = sock;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next V2 1/2] net: introduce skb_coalesce_rx_frag()
From: Jason Wang @ 2013-11-01 5:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: mwdalton, mst, netdev, linux-kernel, virtualization, kmindg,
edumazet, davem
In-Reply-To: <1383229563.4857.71.camel@edumazet-glaptop.roam.corp.google.com>
On 10/31/2013 10:26 PM, Eric Dumazet wrote:
> On Thu, 2013-10-31 at 19:47 +0800, Jason Wang wrote:
>> Sometimes we need to coalesce the rx frags to avoid frag list. One example is
>> virtio-net driver which tries to use small frags for both MTU sized packet and
>> GSO packet. So this patch introduce skb_coalesce_rx_frag() to do this.
>>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Michael Dalton <mwdalton@google.com>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> Changes from V1:
>> - remove the useless off parameter.
>> ---
>> include/linux/skbuff.h | 3 +++
>> net/core/skbuff.c | 13 +++++++++++++
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 2c15497..fffaeaf 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -1372,6 +1372,9 @@ static inline void skb_fill_page_desc(struct sk_buff *skb, int i,
>> void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
>> int size, unsigned int truesize);
>>
>> +void skb_coalesce_rx_frag(struct sk_buff *skb, int i, int size,
>> + unsigned int truesize);
>> +
>> #define SKB_PAGE_ASSERT(skb) BUG_ON(skb_shinfo(skb)->nr_frags)
>> #define SKB_FRAG_ASSERT(skb) BUG_ON(skb_has_frag_list(skb))
>> #define SKB_LINEAR_ASSERT(skb) BUG_ON(skb_is_nonlinear(skb))
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index 0ab32fa..87670e1 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -476,6 +476,19 @@ void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
>> }
>> EXPORT_SYMBOL(skb_add_rx_frag);
>>
>> +void skb_coalesce_rx_frag(struct sk_buff *skb, int i, int size,
>> + unsigned int truesize)
>> +{
>> + skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>> +
>> + skb_frag_size_add(frag, size);
>> + skb->len += size;
>> + skb->data_len += size;
>> + skb->truesize += truesize;
>
>> + skb_frag_unref(skb, i);
> This unref is not logical, or should at least be
>
> __skb_frag_unref(frag);
>
> But I do think this is best done in the caller.
>
> In virtio_net this would be a :
>
> put_page(page);
>
> In tcp stack we do almost the same, but we take the reference on the
> page if we could not coalesce with prio frag, instead of doing a get and
> put in the other case.
>
> if (can_coalesce) {
> skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
> } else {
> get_page(page);
> skb_fill_page_desc(skb, i, page, offset, copy);
> }
>
Ok, get it. Will do a put_page() in V3.
Thanks
>> +}
>> +EXPORT_SYMBOL(skb_coalesce_rx_frag);
>> +
>> static void skb_drop_list(struct sk_buff **listp)
>> {
>> kfree_skb_list(*listp);
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next 2/2] virtio-net: coalesce rx frags when possible during rx
From: Jason Wang @ 2013-11-01 5:37 UTC (permalink / raw)
To: Rusty Russell, davem, edumazet, linux-kernel, netdev, mst,
mwdalton, virtualization
In-Reply-To: <87iowcc1id.fsf@rustcorp.com.au>
On 11/01/2013 10:13 AM, Rusty Russell wrote:
> Jason Wang <jasowang@redhat.com> writes:
>> Commit 2613af0ed18a11d5c566a81f9a6510b73180660a (virtio_net: migrate mergeable
>> rx buffers to page frag allocators) try to increase the payload/truesize for
>> MTU-sized traffic. But this will introduce the extra overhead for GSO packets
>> received because of the frag list. This commit tries to reduce this issue by
>> coalesce the possible rx frags when possible during rx. Test result shows the
>> about 15% improvement on full size GSO packet receiving (and even better than
>> commit 2613af0ed18a11d5c566a81f9a6510b73180660a).
> I don't know about the other users of skb_add_rx_frag, but should
> this coalesce-if-possible code be built into that?
There're several other users without the possibility check. And Eric
point out in do_tcp_sendpages() skb_add_rx_frag() and skb_can_coalesce()
were used separatedly which can allows us to do some other things in the
middle. So it was ok here.
>
> Thanks,
> Rusty.
^ permalink raw reply
* [PATCH net-next V3 1/2] net: introduce skb_coalesce_rx_frag()
From: Jason Wang @ 2013-11-01 6:07 UTC (permalink / raw)
To: davem, edumazet, linux-kernel, netdev, rusty, mst, mwdalton,
virtualization
Cc: kmindg
Sometimes we need to coalesce the rx frags to avoid frag list. One example is
virtio-net driver which tries to use small frags for both MTU sized packet and
GSO packet. So this patch introduce skb_coalesce_rx_frag() to do this.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Michael Dalton <mwdalton@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from V2:
- remove the skb_frag_unref() and let the called to put the page reference
Changes from V1:
- remove the useless off parameter.
---
include/linux/skbuff.h | 3 +++
net/core/skbuff.c | 12 ++++++++++++
2 files changed, 15 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2c15497..fffaeaf 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1372,6 +1372,9 @@ static inline void skb_fill_page_desc(struct sk_buff *skb, int i,
void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
int size, unsigned int truesize);
+void skb_coalesce_rx_frag(struct sk_buff *skb, int i, int size,
+ unsigned int truesize);
+
#define SKB_PAGE_ASSERT(skb) BUG_ON(skb_shinfo(skb)->nr_frags)
#define SKB_FRAG_ASSERT(skb) BUG_ON(skb_has_frag_list(skb))
#define SKB_LINEAR_ASSERT(skb) BUG_ON(skb_is_nonlinear(skb))
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0ab32fa..a729b97 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -476,6 +476,18 @@ void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
}
EXPORT_SYMBOL(skb_add_rx_frag);
+void skb_coalesce_rx_frag(struct sk_buff *skb, int i, int size,
+ unsigned int truesize)
+{
+ skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+ skb_frag_size_add(frag, size);
+ skb->len += size;
+ skb->data_len += size;
+ skb->truesize += truesize;
+}
+EXPORT_SYMBOL(skb_coalesce_rx_frag);
+
static void skb_drop_list(struct sk_buff **listp)
{
kfree_skb_list(*listp);
--
1.8.1.2
^ permalink raw reply related
* [PATCH net-next V3 2/2] virtio-net: coalesce rx frags when possible during rx
From: Jason Wang @ 2013-11-01 6:07 UTC (permalink / raw)
To: davem, edumazet, linux-kernel, netdev, rusty, mst, mwdalton,
virtualization
Cc: kmindg
In-Reply-To: <1383286068-10421-1-git-send-email-jasowang@redhat.com>
Commit 2613af0ed18a11d5c566a81f9a6510b73180660a (virtio_net: migrate mergeable
rx buffers to page frag allocators) try to increase the payload/truesize for
MTU-sized traffic. But this will introduce the extra overhead for GSO packets
received because of the frag list. This commit tries to reduce this issue by
coalesce the possible rx frags when possible during rx. Test result shows the
about 15% improvement on full size GSO packet receiving (and even better than
before commit 2613af0ed18a11d5c566a81f9a6510b73180660a).
Before this commit:
./netperf -H 192.168.100.4
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.100.4
() port 0 AF_INET : demo
Recv Send Send
Socket Socket Message Elapsed
Size Size Size Time Throughput
bytes bytes bytes secs. 10^6bits/sec
87380 16384 16384 10.00 20303.87
After this commit:
./netperf -H 192.168.100.4
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.100.4
() port 0 AF_INET : demo
Recv Send Send
Socket Socket Message Elapsed
Size Size Size Time Throughput
bytes bytes bytes secs. 10^6bits/sec
87380 16384 16384 10.00 23841.26
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Michael Dalton <mwdalton@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from V2:
- call put_page() instead of depending on skb_coalesce_rx_frag()
---
drivers/net/virtio_net.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 113ee93..50774ae 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -305,7 +305,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
struct sk_buff *curr_skb = head_skb;
char *buf;
struct page *page;
- int num_buf, len;
+ int num_buf, len, offset;
num_buf = hdr->mhdr.num_buffers;
while (--num_buf) {
@@ -342,9 +342,16 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
head_skb->truesize += MAX_PACKET_LEN;
}
page = virt_to_head_page(buf);
- skb_add_rx_frag(curr_skb, num_skb_frags, page,
- buf - (char *)page_address(page), len,
- MAX_PACKET_LEN);
+ offset = buf - (char *)page_address(page);
+ if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
+ put_page(page);
+ skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
+ len, MAX_PACKET_LEN);
+ } else {
+ skb_add_rx_frag(curr_skb, num_skb_frags, page,
+ offset, len,
+ MAX_PACKET_LEN);
+ }
--rq->num;
}
return 0;
--
1.8.1.2
^ permalink raw reply related
* [PATCH net] net: flow_dissector: fail on evil iph->ihl
From: Jason Wang @ 2013-11-01 7:01 UTC (permalink / raw)
To: davem, edumazet, netdev, linux-kernel
Cc: Jason Wang, Petr Matousek, Michael S. Tsirkin, Daniel Borkmann
We don't validate iph->ihl which may lead a dead loop if we meet a IPIP
skb whose iph->ihl is zero. Fix this by failing immediately when iph->ihl
is evil (less than 5).
This issue were introduced by commit ec5efe7946280d1e84603389a1030ccec0a767ae
(rps: support IPIP encapsulation).
Cc: Eric Dumazet <edumazet@google.com>
Cc: Petr Matousek <pmatouse@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
This patch is needed for stable.
---
net/core/flow_dissector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 8d7d0dd..143b6fd 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -40,7 +40,7 @@ again:
struct iphdr _iph;
ip:
iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
- if (!iph)
+ if (!iph || iph->ihl < 5)
return false;
if (ip_is_fragment(iph))
--
1.8.1.2
^ permalink raw reply related
* pull request (net-next): ipsec-next 2013-11-01
From: Steffen Klassert @ 2013-11-01 7:50 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
This pull request has a trivial merge conflict between git commit
e7d8f6c ("xfrm: Add refcount handling to queued policies") and
git commit 4d53eff ("xfrm: Don't queue retransmitted packets if
the original is still on the host"). It can be solved as it is
currently done in linux-next.
1) Use vmalloc_node for ipcomp percpu scratches to get proper
NUMA affinity, from Eric Dumazet
2) Avoid to retransmit packets if the original packet is
still in the xfrm hold queue.
3) Remove unused padlen field from struct esp_data.
From Mathias Krause.
4) Remove struct esp_data because now it consists of a single
pointer. From Mathias Krause.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit 5bf47256f5433a9a5782754664e5ea04acb0537d:
Merge tag 'batman-adv-for-davem' of git://git.open-mesh.org/linux-merge (2013-10-19 19:52:42 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
for you to fetch changes up to 1c5ad13f7c2b2afe30e43858d04fff979dc9d243:
net: esp{4,6}: get rid of struct esp_data (2013-10-29 06:39:42 +0100)
----------------------------------------------------------------
Eric Dumazet (1):
xfrm: use vmalloc_node() for percpu scratches
Mathias Krause (2):
net: esp{4,6}: remove padlen from struct esp_data
net: esp{4,6}: get rid of struct esp_data
Steffen Klassert (1):
xfrm: Don't queue retransmitted packets if the original is still on the host
include/net/esp.h | 10 ----------
net/ipv4/esp4.c | 49 +++++++++++++++---------------------------------
net/ipv6/esp6.c | 48 +++++++++++++++--------------------------------
net/xfrm/xfrm_ipcomp.c | 6 ++++--
net/xfrm/xfrm_policy.c | 7 +++++++
5 files changed, 41 insertions(+), 79 deletions(-)
^ permalink raw reply
* [PATCH 1/4] xfrm: use vmalloc_node() for percpu scratches
From: Steffen Klassert @ 2013-11-01 7:50 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1383292257-4634-1-git-send-email-steffen.klassert@secunet.com>
From: Eric Dumazet <edumazet@google.com>
scratches are per cpu, we can use vmalloc_node() for proper
NUMA affinity.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_ipcomp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index 2906d52..b943c7f 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -220,8 +220,8 @@ static void ipcomp_free_scratches(void)
static void * __percpu *ipcomp_alloc_scratches(void)
{
- int i;
void * __percpu *scratches;
+ int i;
if (ipcomp_scratch_users++)
return ipcomp_scratches;
@@ -233,7 +233,9 @@ static void * __percpu *ipcomp_alloc_scratches(void)
ipcomp_scratches = scratches;
for_each_possible_cpu(i) {
- void *scratch = vmalloc(IPCOMP_SCRATCH_SIZE);
+ void *scratch;
+
+ scratch = vmalloc_node(IPCOMP_SCRATCH_SIZE, cpu_to_node(i));
if (!scratch)
return NULL;
*per_cpu_ptr(scratches, i) = scratch;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/4] xfrm: Don't queue retransmitted packets if the original is still on the host
From: Steffen Klassert @ 2013-11-01 7:50 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1383292257-4634-1-git-send-email-steffen.klassert@secunet.com>
It does not make sense to queue retransmitted packets if the
original packet is still in some queue of this host. So add
a check to xdst_queue_output() and drop the packet if the
original packet is not yet sent.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
net/xfrm/xfrm_policy.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index ed38d5d..e09edfc 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1832,6 +1832,13 @@ static int xdst_queue_output(struct sk_buff *skb)
struct dst_entry *dst = skb_dst(skb);
struct xfrm_dst *xdst = (struct xfrm_dst *) dst;
struct xfrm_policy_queue *pq = &xdst->pols[0]->polq;
+ const struct sk_buff *fclone = skb + 1;
+
+ if (unlikely(skb->fclone == SKB_FCLONE_ORIG &&
+ fclone->fclone == SKB_FCLONE_CLONE)) {
+ kfree_skb(skb);
+ return 0;
+ }
if (pq->hold_queue.qlen > XFRM_MAX_QUEUE_LEN) {
kfree_skb(skb);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/4] net: esp{4,6}: remove padlen from struct esp_data
From: Steffen Klassert @ 2013-11-01 7:50 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1383292257-4634-1-git-send-email-steffen.klassert@secunet.com>
From: Mathias Krause <mathias.krause@secunet.com>
The padlen member of struct esp_data is always zero. Get rid of it.
Signed-off-by: Mathias Krause <mathias.krause@secunet.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/esp.h | 3 ---
net/ipv4/esp4.c | 9 +--------
net/ipv6/esp6.c | 9 +--------
3 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/include/net/esp.h b/include/net/esp.h
index 1356dda..706b740 100644
--- a/include/net/esp.h
+++ b/include/net/esp.h
@@ -6,9 +6,6 @@
struct crypto_aead;
struct esp_data {
- /* 0..255 */
- int padlen;
-
/* Confidentiality & Integrity */
struct crypto_aead *aead;
};
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 109ee89..8b5386a 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -154,8 +154,6 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
}
blksize = ALIGN(crypto_aead_blocksize(aead), 4);
clen = ALIGN(skb->len + 2 + tfclen, blksize);
- if (esp->padlen)
- clen = ALIGN(clen, esp->padlen);
plen = clen - skb->len - tfclen;
err = skb_cow_data(skb, tfclen + plen + alen, &trailer);
@@ -461,7 +459,6 @@ static u32 esp4_get_mtu(struct xfrm_state *x, int mtu)
{
struct esp_data *esp = x->data;
u32 blksize = ALIGN(crypto_aead_blocksize(esp->aead), 4);
- u32 align = max_t(u32, blksize, esp->padlen);
unsigned int net_adj;
switch (x->props.mode) {
@@ -477,7 +474,7 @@ static u32 esp4_get_mtu(struct xfrm_state *x, int mtu)
}
return ((mtu - x->props.header_len - crypto_aead_authsize(esp->aead) -
- net_adj) & ~(align - 1)) + net_adj - 2;
+ net_adj) & ~(blksize - 1)) + net_adj - 2;
}
static void esp4_err(struct sk_buff *skb, u32 info)
@@ -659,8 +656,6 @@ static int esp_init_state(struct xfrm_state *x)
aead = esp->aead;
- esp->padlen = 0;
-
x->props.header_len = sizeof(struct ip_esp_hdr) +
crypto_aead_ivsize(aead);
if (x->props.mode == XFRM_MODE_TUNNEL)
@@ -683,8 +678,6 @@ static int esp_init_state(struct xfrm_state *x)
}
align = ALIGN(crypto_aead_blocksize(aead), 4);
- if (esp->padlen)
- align = max_t(u32, align, esp->padlen);
x->props.trailer_len = align + 1 + crypto_aead_authsize(esp->aead);
error:
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index d3618a7..0073cd0 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -181,8 +181,6 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
}
blksize = ALIGN(crypto_aead_blocksize(aead), 4);
clen = ALIGN(skb->len + 2 + tfclen, blksize);
- if (esp->padlen)
- clen = ALIGN(clen, esp->padlen);
plen = clen - skb->len - tfclen;
err = skb_cow_data(skb, tfclen + plen + alen, &trailer);
@@ -416,7 +414,6 @@ static u32 esp6_get_mtu(struct xfrm_state *x, int mtu)
{
struct esp_data *esp = x->data;
u32 blksize = ALIGN(crypto_aead_blocksize(esp->aead), 4);
- u32 align = max_t(u32, blksize, esp->padlen);
unsigned int net_adj;
if (x->props.mode != XFRM_MODE_TUNNEL)
@@ -425,7 +422,7 @@ static u32 esp6_get_mtu(struct xfrm_state *x, int mtu)
net_adj = 0;
return ((mtu - x->props.header_len - crypto_aead_authsize(esp->aead) -
- net_adj) & ~(align - 1)) + net_adj - 2;
+ net_adj) & ~(blksize - 1)) + net_adj - 2;
}
static void esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
@@ -606,8 +603,6 @@ static int esp6_init_state(struct xfrm_state *x)
aead = esp->aead;
- esp->padlen = 0;
-
x->props.header_len = sizeof(struct ip_esp_hdr) +
crypto_aead_ivsize(aead);
switch (x->props.mode) {
@@ -626,8 +621,6 @@ static int esp6_init_state(struct xfrm_state *x)
}
align = ALIGN(crypto_aead_blocksize(aead), 4);
- if (esp->padlen)
- align = max_t(u32, align, esp->padlen);
x->props.trailer_len = align + 1 + crypto_aead_authsize(esp->aead);
error:
--
1.7.9.5
^ permalink raw reply related
* [PATCH 4/4] net: esp{4,6}: get rid of struct esp_data
From: Steffen Klassert @ 2013-11-01 7:50 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1383292257-4634-1-git-send-email-steffen.klassert@secunet.com>
From: Mathias Krause <mathias.krause@secunet.com>
struct esp_data consists of a single pointer, vanishing the need for it
to be a structure. Fold the pointer into 'data' direcly, removing one
level of pointer indirection.
Signed-off-by: Mathias Krause <mathias.krause@secunet.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/esp.h | 7 -------
net/ipv4/esp4.c | 40 ++++++++++++++--------------------------
net/ipv6/esp6.c | 39 ++++++++++++++-------------------------
3 files changed, 28 insertions(+), 58 deletions(-)
diff --git a/include/net/esp.h b/include/net/esp.h
index 706b740..c92213c 100644
--- a/include/net/esp.h
+++ b/include/net/esp.h
@@ -3,13 +3,6 @@
#include <linux/skbuff.h>
-struct crypto_aead;
-
-struct esp_data {
- /* Confidentiality & Integrity */
- struct crypto_aead *aead;
-};
-
void *pskb_put(struct sk_buff *skb, struct sk_buff *tail, int len);
struct ip_esp_hdr;
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 8b5386a..7785b28 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -121,7 +121,6 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
struct aead_givcrypt_request *req;
struct scatterlist *sg;
struct scatterlist *asg;
- struct esp_data *esp;
struct sk_buff *trailer;
void *tmp;
u8 *iv;
@@ -139,8 +138,7 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
/* skb is pure payload to encrypt */
- esp = x->data;
- aead = esp->aead;
+ aead = x->data;
alen = crypto_aead_authsize(aead);
tfclen = 0;
@@ -278,8 +276,7 @@ static int esp_input_done2(struct sk_buff *skb, int err)
{
const struct iphdr *iph;
struct xfrm_state *x = xfrm_input_state(skb);
- struct esp_data *esp = x->data;
- struct crypto_aead *aead = esp->aead;
+ struct crypto_aead *aead = x->data;
int alen = crypto_aead_authsize(aead);
int hlen = sizeof(struct ip_esp_hdr) + crypto_aead_ivsize(aead);
int elen = skb->len - hlen;
@@ -374,8 +371,7 @@ static void esp_input_done(struct crypto_async_request *base, int err)
static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
{
struct ip_esp_hdr *esph;
- struct esp_data *esp = x->data;
- struct crypto_aead *aead = esp->aead;
+ struct crypto_aead *aead = x->data;
struct aead_request *req;
struct sk_buff *trailer;
int elen = skb->len - sizeof(*esph) - crypto_aead_ivsize(aead);
@@ -457,8 +453,8 @@ out:
static u32 esp4_get_mtu(struct xfrm_state *x, int mtu)
{
- struct esp_data *esp = x->data;
- u32 blksize = ALIGN(crypto_aead_blocksize(esp->aead), 4);
+ struct crypto_aead *aead = x->data;
+ u32 blksize = ALIGN(crypto_aead_blocksize(aead), 4);
unsigned int net_adj;
switch (x->props.mode) {
@@ -473,7 +469,7 @@ static u32 esp4_get_mtu(struct xfrm_state *x, int mtu)
BUG();
}
- return ((mtu - x->props.header_len - crypto_aead_authsize(esp->aead) -
+ return ((mtu - x->props.header_len - crypto_aead_authsize(aead) -
net_adj) & ~(blksize - 1)) + net_adj - 2;
}
@@ -508,18 +504,16 @@ static void esp4_err(struct sk_buff *skb, u32 info)
static void esp_destroy(struct xfrm_state *x)
{
- struct esp_data *esp = x->data;
+ struct crypto_aead *aead = x->data;
- if (!esp)
+ if (!aead)
return;
- crypto_free_aead(esp->aead);
- kfree(esp);
+ crypto_free_aead(aead);
}
static int esp_init_aead(struct xfrm_state *x)
{
- struct esp_data *esp = x->data;
struct crypto_aead *aead;
int err;
@@ -528,7 +522,7 @@ static int esp_init_aead(struct xfrm_state *x)
if (IS_ERR(aead))
goto error;
- esp->aead = aead;
+ x->data = aead;
err = crypto_aead_setkey(aead, x->aead->alg_key,
(x->aead->alg_key_len + 7) / 8);
@@ -545,7 +539,6 @@ error:
static int esp_init_authenc(struct xfrm_state *x)
{
- struct esp_data *esp = x->data;
struct crypto_aead *aead;
struct crypto_authenc_key_param *param;
struct rtattr *rta;
@@ -580,7 +573,7 @@ static int esp_init_authenc(struct xfrm_state *x)
if (IS_ERR(aead))
goto error;
- esp->aead = aead;
+ x->data = aead;
keylen = (x->aalg ? (x->aalg->alg_key_len + 7) / 8 : 0) +
(x->ealg->alg_key_len + 7) / 8 + RTA_SPACE(sizeof(*param));
@@ -635,16 +628,11 @@ error:
static int esp_init_state(struct xfrm_state *x)
{
- struct esp_data *esp;
struct crypto_aead *aead;
u32 align;
int err;
- esp = kzalloc(sizeof(*esp), GFP_KERNEL);
- if (esp == NULL)
- return -ENOMEM;
-
- x->data = esp;
+ x->data = NULL;
if (x->aead)
err = esp_init_aead(x);
@@ -654,7 +642,7 @@ static int esp_init_state(struct xfrm_state *x)
if (err)
goto error;
- aead = esp->aead;
+ aead = x->data;
x->props.header_len = sizeof(struct ip_esp_hdr) +
crypto_aead_ivsize(aead);
@@ -678,7 +666,7 @@ static int esp_init_state(struct xfrm_state *x)
}
align = ALIGN(crypto_aead_blocksize(aead), 4);
- x->props.trailer_len = align + 1 + crypto_aead_authsize(esp->aead);
+ x->props.trailer_len = align + 1 + crypto_aead_authsize(aead);
error:
return err;
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 0073cd0..87eb79e 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -164,10 +164,9 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
u8 *iv;
u8 *tail;
__be32 *seqhi;
- struct esp_data *esp = x->data;
/* skb is pure payload to encrypt */
- aead = esp->aead;
+ aead = x->data;
alen = crypto_aead_authsize(aead);
tfclen = 0;
@@ -269,8 +268,7 @@ error:
static int esp_input_done2(struct sk_buff *skb, int err)
{
struct xfrm_state *x = xfrm_input_state(skb);
- struct esp_data *esp = x->data;
- struct crypto_aead *aead = esp->aead;
+ struct crypto_aead *aead = x->data;
int alen = crypto_aead_authsize(aead);
int hlen = sizeof(struct ip_esp_hdr) + crypto_aead_ivsize(aead);
int elen = skb->len - hlen;
@@ -323,8 +321,7 @@ static void esp_input_done(struct crypto_async_request *base, int err)
static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
{
struct ip_esp_hdr *esph;
- struct esp_data *esp = x->data;
- struct crypto_aead *aead = esp->aead;
+ struct crypto_aead *aead = x->data;
struct aead_request *req;
struct sk_buff *trailer;
int elen = skb->len - sizeof(*esph) - crypto_aead_ivsize(aead);
@@ -412,8 +409,8 @@ out:
static u32 esp6_get_mtu(struct xfrm_state *x, int mtu)
{
- struct esp_data *esp = x->data;
- u32 blksize = ALIGN(crypto_aead_blocksize(esp->aead), 4);
+ struct crypto_aead *aead = x->data;
+ u32 blksize = ALIGN(crypto_aead_blocksize(aead), 4);
unsigned int net_adj;
if (x->props.mode != XFRM_MODE_TUNNEL)
@@ -421,7 +418,7 @@ static u32 esp6_get_mtu(struct xfrm_state *x, int mtu)
else
net_adj = 0;
- return ((mtu - x->props.header_len - crypto_aead_authsize(esp->aead) -
+ return ((mtu - x->props.header_len - crypto_aead_authsize(aead) -
net_adj) & ~(blksize - 1)) + net_adj - 2;
}
@@ -452,18 +449,16 @@ static void esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
static void esp6_destroy(struct xfrm_state *x)
{
- struct esp_data *esp = x->data;
+ struct crypto_aead *aead = x->data;
- if (!esp)
+ if (!aead)
return;
- crypto_free_aead(esp->aead);
- kfree(esp);
+ crypto_free_aead(aead);
}
static int esp_init_aead(struct xfrm_state *x)
{
- struct esp_data *esp = x->data;
struct crypto_aead *aead;
int err;
@@ -472,7 +467,7 @@ static int esp_init_aead(struct xfrm_state *x)
if (IS_ERR(aead))
goto error;
- esp->aead = aead;
+ x->data = aead;
err = crypto_aead_setkey(aead, x->aead->alg_key,
(x->aead->alg_key_len + 7) / 8);
@@ -489,7 +484,6 @@ error:
static int esp_init_authenc(struct xfrm_state *x)
{
- struct esp_data *esp = x->data;
struct crypto_aead *aead;
struct crypto_authenc_key_param *param;
struct rtattr *rta;
@@ -524,7 +518,7 @@ static int esp_init_authenc(struct xfrm_state *x)
if (IS_ERR(aead))
goto error;
- esp->aead = aead;
+ x->data = aead;
keylen = (x->aalg ? (x->aalg->alg_key_len + 7) / 8 : 0) +
(x->ealg->alg_key_len + 7) / 8 + RTA_SPACE(sizeof(*param));
@@ -579,7 +573,6 @@ error:
static int esp6_init_state(struct xfrm_state *x)
{
- struct esp_data *esp;
struct crypto_aead *aead;
u32 align;
int err;
@@ -587,11 +580,7 @@ static int esp6_init_state(struct xfrm_state *x)
if (x->encap)
return -EINVAL;
- esp = kzalloc(sizeof(*esp), GFP_KERNEL);
- if (esp == NULL)
- return -ENOMEM;
-
- x->data = esp;
+ x->data = NULL;
if (x->aead)
err = esp_init_aead(x);
@@ -601,7 +590,7 @@ static int esp6_init_state(struct xfrm_state *x)
if (err)
goto error;
- aead = esp->aead;
+ aead = x->data;
x->props.header_len = sizeof(struct ip_esp_hdr) +
crypto_aead_ivsize(aead);
@@ -621,7 +610,7 @@ static int esp6_init_state(struct xfrm_state *x)
}
align = ALIGN(crypto_aead_blocksize(aead), 4);
- x->props.trailer_len = align + 1 + crypto_aead_authsize(esp->aead);
+ x->props.trailer_len = align + 1 + crypto_aead_authsize(aead);
error:
return err;
--
1.7.9.5
^ permalink raw reply related
* Re: 3.12-rc7 regression - network panic from ipv6
From: Steffen Klassert @ 2013-11-01 7:56 UTC (permalink / raw)
To: mroos; +Cc: David Miller, hannes, Linux Kernel list, netdev
In-Reply-To: <alpine.SOC.1.00.1310302340170.24516@math.ut.ee>
On Wed, Oct 30, 2013 at 11:41:09PM +0200, mroos@linux.ee wrote:
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>
> Works fine, thanks!
>
> Tested-by: Meelis Roos <mroos@linux.ee>
>
I've applied the fix now to the ipsec tree. Again thanks for
reporting and testing!
^ permalink raw reply
* pull request (net): ipsec 2013-11-01
From: Steffen Klassert @ 2013-11-01 8:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
1) Fix a possible race on ipcomp scratch buffers because
of too early enabled siftirqs. From Michal Kubecek.
2) The current xfrm garbage collector threshold is too small
for some workloads, resulting in bad performance on these
workloads. Increase the threshold from 1024 to 32768.
3) Some codepaths might not have a dst_entry attached to the
skb when calling xfrm_decode_session(). So add a check
to prevent a null pointer dereference in this case.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit e9e4ea74f06635f2ffc1dffe5ef40c854faa0a90:
net: smc91x: dont't use SMC_outw for fixing up halfword-aligned data (2013-10-11 17:50:59 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master
for you to fetch changes up to 84502b5ef9849a9694673b15c31bd3ac693010ae:
xfrm: Fix null pointer dereference when decoding sessions (2013-11-01 07:08:46 +0100)
----------------------------------------------------------------
Michal Kubecek (1):
xfrm: prevent ipcomp scratch buffer race condition
Steffen Klassert (2):
xfrm: Increase the garbage collector threshold
xfrm: Fix null pointer dereference when decoding sessions
net/ipv4/xfrm4_policy.c | 8 ++++++--
net/ipv6/xfrm6_policy.c | 8 ++++++--
net/xfrm/xfrm_ipcomp.c | 12 ++++++------
3 files changed, 18 insertions(+), 10 deletions(-)
^ permalink raw reply
* [PATCH 1/3] xfrm: prevent ipcomp scratch buffer race condition
From: Steffen Klassert @ 2013-11-01 8:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1383294107-7509-1-git-send-email-steffen.klassert@secunet.com>
From: Michal Kubecek <mkubecek@suse.cz>
In ipcomp_compress(), sortirq is enabled too early, allowing the
per-cpu scratch buffer to be rewritten by ipcomp_decompress()
(called on the same CPU in softirq context) between populating
the buffer and copying the compressed data to the skb.
v2: as pointed out by Steffen Klassert, if we also move the
local_bh_disable() before reading the per-cpu pointers, we can
get rid of get_cpu()/put_cpu().
v3: removed ipcomp_decompress part (as explained by Herbert Xu,
it cannot be called from process context), get rid of cpu
variable (thanks to Eric Dumazet)
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_ipcomp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index 2906d52..3be02b6 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -141,14 +141,14 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
const int plen = skb->len;
int dlen = IPCOMP_SCRATCH_SIZE;
u8 *start = skb->data;
- const int cpu = get_cpu();
- u8 *scratch = *per_cpu_ptr(ipcomp_scratches, cpu);
- struct crypto_comp *tfm = *per_cpu_ptr(ipcd->tfms, cpu);
+ struct crypto_comp *tfm;
+ u8 *scratch;
int err;
local_bh_disable();
+ scratch = *this_cpu_ptr(ipcomp_scratches);
+ tfm = *this_cpu_ptr(ipcd->tfms);
err = crypto_comp_compress(tfm, start, plen, scratch, &dlen);
- local_bh_enable();
if (err)
goto out;
@@ -158,13 +158,13 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
}
memcpy(start + sizeof(struct ip_comp_hdr), scratch, dlen);
- put_cpu();
+ local_bh_enable();
pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr));
return 0;
out:
- put_cpu();
+ local_bh_enable();
return err;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
From: Steffen Klassert @ 2013-11-01 8:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1383294107-7509-1-git-send-email-steffen.klassert@secunet.com>
On some codepaths the skb does not have a dst entry
when xfrm_decode_session() is called. So check for
a valid skb_dst() before dereferencing the device
interface index. We use 0 as the device index if
there is no valid skb_dst(), or at reverse decoding
we use skb_iif as device interface index.
Bug was introduced with git commit bafd4bd4dc
("xfrm: Decode sessions with output interface.").
Reported-by: Meelis Roos <mroos@linux.ee>
Tested-by: Meelis Roos <mroos@linux.ee>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/xfrm4_policy.c | 6 +++++-
net/ipv6/xfrm6_policy.c | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 4764ee4..e1a6393 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -104,10 +104,14 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
const struct iphdr *iph = ip_hdr(skb);
u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
struct flowi4 *fl4 = &fl->u.ip4;
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl4, 0, sizeof(struct flowi4));
fl4->flowi4_mark = skb->mark;
- fl4->flowi4_oif = skb_dst(skb)->dev->ifindex;
+ fl4->flowi4_oif = reverse ? skb->skb_iif : oif;
if (!ip_is_fragment(iph)) {
switch (iph->protocol) {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index dd503a3..5f8e128 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -135,10 +135,14 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
struct ipv6_opt_hdr *exthdr;
const unsigned char *nh = skb_network_header(skb);
u8 nexthdr = nh[IP6CB(skb)->nhoff];
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl6, 0, sizeof(struct flowi6));
fl6->flowi6_mark = skb->mark;
- fl6->flowi6_oif = skb_dst(skb)->dev->ifindex;
+ fl6->flowi6_oif = reverse ? skb->skb_iif : oif;
fl6->daddr = reverse ? hdr->saddr : hdr->daddr;
fl6->saddr = reverse ? hdr->daddr : hdr->saddr;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/3] xfrm: Increase the garbage collector threshold
From: Steffen Klassert @ 2013-11-01 8:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1383294107-7509-1-git-send-email-steffen.klassert@secunet.com>
With the removal of the routing cache, we lost the
option to tweak the garbage collector threshold
along with the maximum routing cache size. So git
commit 703fb94ec ("xfrm: Fix the gc threshold value
for ipv4") moved back to a static threshold.
It turned out that the current threshold before we
start garbage collecting is much to small for some
workloads, so increase it from 1024 to 32768. This
means that we start the garbage collector if we have
more than 32768 dst entries in the system and refuse
new allocations if we are above 65536.
Reported-by: Wolfgang Walter <linux@stwm.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/xfrm4_policy.c | 2 +-
net/ipv6/xfrm6_policy.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index ccde542..4764ee4 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -236,7 +236,7 @@ static struct dst_ops xfrm4_dst_ops = {
.destroy = xfrm4_dst_destroy,
.ifdown = xfrm4_dst_ifdown,
.local_out = __ip_local_out,
- .gc_thresh = 1024,
+ .gc_thresh = 32768,
};
static struct xfrm_policy_afinfo xfrm4_policy_afinfo = {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 08ed277..dd503a3 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -285,7 +285,7 @@ static struct dst_ops xfrm6_dst_ops = {
.destroy = xfrm6_dst_destroy,
.ifdown = xfrm6_dst_ifdown,
.local_out = __ip6_local_out,
- .gc_thresh = 1024,
+ .gc_thresh = 32768,
};
static struct xfrm_policy_afinfo xfrm6_policy_afinfo = {
--
1.7.9.5
^ permalink raw reply related
* Re: ipv6 fragmentation-related panic in netfilter
From: Steffen Klassert @ 2013-11-01 8:45 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Tomas Hlavacek, netdev, netfilter-devel
In-Reply-To: <20131030000701.GB25469@macbook.localnet>
On Wed, Oct 30, 2013 at 12:07:11AM +0000, Patrick McHardy wrote:
>
> The problem is that the reassembled packet is referenced by the individual
> fragments, so we trigger the BUG_ON in pskb_expand_head(). In this
> particular case the case we BUG() on is actually OK, but I'm looking at
> a way we can fix this without special casing. Hope to have a patch for
> testing in the next hours.
Just for the record. I'm observing similar, quite reproducable crashes when
receiving fragmented icmp echo request packets on an IPsec gateway with
nf_conntrack_ipv6.
Since git commit 58a317f10 ("netfilter: ipv6: add IPv6 NAT support")
netfilter might insert a reassembled ipv6 packet with a shared skb and
local_df = 1 to the ok function. In case of xfrm, __xfrm6_output()
fragments the packet again and when adjusting the headroom later, we
crash because of a shared skb.
I can fix it by checking for a shared skb in ip6_fragment() and do
slow path fragmentation then. But we never needed such a check in
ip6_fragment(), so it's maybe better to fix it in netfilter.
^ permalink raw reply
* Re: [net-next PATCH V2] net: codel: Avoid undefined behavior from signed overflow
From: Paul E. McKenney @ 2013-11-01 8:50 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: netdev, Eric Dumazet, Dave Taht
In-Reply-To: <20131031211055.10355.98182.stgit@dragon>
On Thu, Oct 31, 2013 at 10:10:55PM +0100, Jesper Dangaard Brouer wrote:
> From: Jesper Dangaard Brouer <netoptimizer@brouer.com>
>
> As described in commit 5a581b367 (jiffies: Avoid undefined
> behavior from signed overflow), according to the C standard
> 3.4.3p3, overflow of a signed integer results in undefined
> behavior.
>
> To fix this, do as the above commit, and do an unsigned
> subtraction, and interpreting the result as a signed
> two's-complement number. This is based on the theory from
> RFC 1982 and is nicely described in wikipedia here:
> https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution
>
> A side-note, I have seen practical issues with the previous logic
> when dealing with 16-bit, on a 64-bit machine (gcc version
> 4.4.5). This were 32-bit, which I have not observed issues with.
>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com>
Looks good to me!
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>
> include/net/codel.h | 19 +++++++++++++++----
> 1 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/codel.h b/include/net/codel.h
> index 389cf62..3b04ff5 100644
> --- a/include/net/codel.h
> +++ b/include/net/codel.h
> @@ -72,10 +72,21 @@ static inline codel_time_t codel_get_time(void)
> return ns >> CODEL_SHIFT;
> }
>
> -#define codel_time_after(a, b) ((s32)(a) - (s32)(b) > 0)
> -#define codel_time_after_eq(a, b) ((s32)(a) - (s32)(b) >= 0)
> -#define codel_time_before(a, b) ((s32)(a) - (s32)(b) < 0)
> -#define codel_time_before_eq(a, b) ((s32)(a) - (s32)(b) <= 0)
> +/* Dealing with timer wrapping, according to RFC 1982, as desc in wikipedia:
> + * https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution
> + * codel_time_after(a,b) returns true if the time a is after time b.
> + */
> +#define codel_time_after(a, b) \
> + (typecheck(codel_time_t, a) && \
> + typecheck(codel_time_t, b) && \
> + ((s32)((a) - (b)) > 0))
> +#define codel_time_before(a, b) codel_time_after(b, a)
> +
> +#define codel_time_after_eq(a, b) \
> + (typecheck(codel_time_t, a) && \
> + typecheck(codel_time_t, b) && \
> + ((s32)((a) - (b)) >= 0))
> +#define codel_time_before_eq(a, b) codel_time_after_eq(b, a)
>
> /* Qdiscs using codel plugin must use codel_skb_cb in their own cb[] */
> struct codel_skb_cb {
>
^ permalink raw reply
* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Ingo Molnar @ 2013-11-01 9:13 UTC (permalink / raw)
To: Neil Horman
Cc: Eric Dumazet, linux-kernel, sebastien.dugue, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86, netdev
In-Reply-To: <20131031143325.GB25894@hmsreliant.think-freely.org>
* Neil Horman <nhorman@tuxdriver.com> wrote:
> On Thu, Oct 31, 2013 at 11:22:00AM +0100, Ingo Molnar wrote:
> >
> > * Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > > > etc. For such short runtimes make sure the last column displays
> > > > close to 100%, so that the PMU results become trustable.
> > > >
> > > > A nehalem+ PMU will allow 2-4 events to be measured in parallel,
> > > > plus generics like 'cycles', 'instructions' can be added 'for free'
> > > > because they get counted in a separate (fixed purpose) PMU register.
> > > >
> > > > The last colum tells you what percentage of the runtime that
> > > > particular event was actually active. 100% (or empty last column)
> > > > means it was active all the time.
> > > >
> > > > Thanks,
> > > >
> > > > Ingo
> > > >
> > >
> > > Hmm,
> > >
> > > I ran this test:
> > >
> > > for i in `seq 0 1 3`
> > > do
> > > echo $i > /sys/module/csum_test/parameters/module_test_mode
> > > taskset -c 0 perf stat --repeat 20 -C 0 -e L1-dcache-load-misses -e L1-dcache-prefetches -e cycles -e instructions -ddd ./test.sh
> > > done
> >
> > You need to remove '-ddd' which is a shortcut for a ton of useful
> > events, but here you want to use fewer events, to increase the
> > precision of the measurement.
> >
> > Thanks,
> >
> > Ingo
> >
>
> Thank you ingo, that fixed it. I'm trying some other variants of
> the csum algorithm that Doug and I discussed last night, but FWIW,
> the relative performance of the 4 test cases
> (base/prefetch/parallel/both) remains unchanged. I'm starting to
> feel like at this point, theres very little point in doing
> parallel alu operations (unless we can find a way to break the
> dependency on the carry flag, which is what I'm tinkering with
> now).
I would still like to encourage you to pick up the improvements that
Doug measured (mostly via prefetch tweaking?) - that looked like
some significant speedups that we don't want to lose!
Also, trying to stick the in-kernel implementation into 'perf bench'
would be a useful first step as well, for this and future efforts.
See what we do in tools/perf/bench/mem-memcpy-x86-64-asm.S to pick
up the in-kernel assembly memcpy implementations:
#define memcpy MEMCPY /* don't hide glibc's memcpy() */
#define altinstr_replacement text
#define globl p2align 4; .globl
#define Lmemcpy_c globl memcpy_c; memcpy_c
#define Lmemcpy_c_e globl memcpy_c_e; memcpy_c_e
#include "../../../arch/x86/lib/memcpy_64.S"
So it needed a bit of trickery/wrappery for 'perf bench mem memcpy',
but that is a one-time effort - once it's done then the current
in-kernel csum_partial() implementation would be easily measurable
(and any performance regression in it bisectable, etc.) from that
point on.
In user-space it would also be easier to add various parameters and
experimental implementations and background cache-stressing
workloads automatically.
Something similar might be possible for csum_partial(),
csum_partial_copy*(), etc.
Note, if any of you ventures to add checksum-benchmarking to perf
bench, please base any patches on top of tip:perf/core:
git pull git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core
as there are a couple of perf bench enhancements in the pipeline
already for v3.13.
Thanks,
Ingo
^ 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