Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] net: Fix potential memory leak
From: Eric Dumazet @ 2011-09-26  6:09 UTC (permalink / raw)
  To: Huajun Li; +Cc: David Miller, netdev
In-Reply-To: <CA+v9cxYuFArVUh9gc-be-Lmy4TOTxp72VaCC4QeCEE3p1j2Ykg@mail.gmail.com>

Le samedi 24 septembre 2011 à 23:57 +0800, Huajun Li a écrit :
> While preparing flow caches, once fail may cause potential memory leak , fix it.
> 
> Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
> ---
>  net/core/flow.c |   19 ++++++++++++++++++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/net/core/flow.c b/net/core/flow.c
> index ba3e617..2dcaa03 100644
> --- a/net/core/flow.c
> +++ b/net/core/flow.c
> @@ -420,7 +420,7 @@ static int __init flow_cache_init(struct flow_cache *fc)
> 
>  	for_each_online_cpu(i) {
>  		if (flow_cache_cpu_prepare(fc, i))
> -			return -ENOMEM;
> +			goto err;
>  	}
>  	fc->hotcpu_notifier = (struct notifier_block){
>  		.notifier_call = flow_cache_cpu,
> @@ -433,6 +433,23 @@ static int __init flow_cache_init(struct flow_cache *fc)
>  	add_timer(&fc->rnd_timer);
> 
>  	return 0;
> +err:
> +	if (fc->percpu) {
> +		free_percpu(fc->percpu);
> +		fc->percpu = NULL;
> +	}
> +
> +	/*
> +	 * Check each possible CPUs rather than online ones because they may be
> +	 * offline before the notifier is registered.
> +	 */

Please remove this comment.


> +	for_each_possible_cpu(i) {
> +		struct flow_cache_percpu *fcp = per_cpu_ptr(fc->percpu, i);
> +		kfree(fcp->hash_table);
> +		fcp->hash_table = NULL;
> +	}

You access fc->percpu after freeing it...

> +
> +	return -ENOMEM;
>  }
> 
>  static int __init flow_cache_init_global(void)

Previous to 2.6.37 (commit 83b6b1f5d134), a memory allocation at this
stage was panicing the box, so no worry about mem leak :)

Now I wonder if a proper patch would not print a nice message in
flow_cache_init_global() if flow_cache_init() returns an error, instead
of silently panicing or something worse...

Before submitting a new patch, could you test this case (injecting a
memalloc error in flow_cache_cpu_prepare() for example.

^ permalink raw reply

* Re: [PATCH] tcp: properly release md5sig_pool references
From: Eric Dumazet @ 2011-09-26  6:26 UTC (permalink / raw)
  To: Yan, Zheng; +Cc: netdev@vger.kernel.org, davem@davemloft.net, yoshfuji
In-Reply-To: <4E7FE4DD.5070804@intel.com>

Le lundi 26 septembre 2011 à 10:35 +0800, Yan, Zheng a écrit :
> tcp_v4_md5_do_add() increases use count of md5sig_pool by one for
> each tcp md5sig peer, so we should call tcp_free_md5sig_pool() for
> each peer when destroying the socket.
> 
> Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>

As managing the use count is rather expensive, I think it would be
better to hold a single reference instead of multiple ones ?

^ permalink raw reply

* Re: [PATCH 0/4] IPVS: sysctl Documentation and Various Clean-Ups
From: Hans Schillstrom @ 2011-09-26  6:43 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, netfilter, Wensong Zhang,
	Julian Anastasov, Patrick McHardy
In-Reply-To: <1317000233-11169-1-git-send-email-horms@verge.net.au>

Hello Simon

On Monday, September 26, 2011 03:23:49 Simon Horman wrote:
> Hi,
> 
> patch one of this series adds some missing entries to the sysctl
> documentation. Julian, I would appreciate a review of this change.
> 
> The remaining three patches are minor code clean-ups flagged
> by gcc 4.6.1.
> 
> My intention for this post is to solicit review.
> I intend to post a pull request separately at a later date.
> 

Your patches looks fine to me,
you can have mine ack as well

Acked-by Hans Schillstrom <hans@schillstrom.com>

I have two small pending patches,  Deadlock and lockdep fixes. 
Hopefully the patches will be ready this week. The deadlock is a tricky bastard to fix. 

Regards
Hans

^ permalink raw reply

* [PATCH 1/2] pch_gbe: Fixed the issue on which PC was frozen when link was downed.
From: Toshiharu Okada @ 2011-09-26  7:27 UTC (permalink / raw)
  To: netdev, linux-kernel, eric.dumazet, davem
  Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, tomoya-linux,
	Toshiharu Okada

When a link was downed during network use,
there is an issue on which PC freezes.

This patch fixed this issue.

Signed-off-by: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com>
---
 drivers/net/pch_gbe/pch_gbe_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/pch_gbe/pch_gbe_main.c b/drivers/net/pch_gbe/pch_gbe_main.c
index 567ff10..5474189 100644
--- a/drivers/net/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/pch_gbe/pch_gbe_main.c
@@ -2195,7 +2195,7 @@ static int pch_gbe_napi_poll(struct napi_struct *napi, int budget)
 		/* If no Tx and not enough Rx work done,
 		 * exit the polling mode
 		 */
-		if ((work_done < budget) || !netif_running(netdev))
+		if (work_done < budget)
 			poll_end_flag = true;
 	}
 
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 2/2] pch_gbe: Fixed the issue on which a network freezes
From: Toshiharu Okada @ 2011-09-26  7:27 UTC (permalink / raw)
  To: netdev, linux-kernel, eric.dumazet, davem
  Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, tomoya-linux,
	Toshiharu Okada
In-Reply-To: <1317022063-12143-1-git-send-email-toshiharu-linux@dsn.okisemi.com>

The pch_gbe driver has an issue which a network stops,
when receiving traffic is high.
In the case, The link down and up are necessary to return a network.

This patch fixed this issue.

Signed-off-by: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com>
---
 drivers/net/pch_gbe/pch_gbe_main.c |   56 +++++++++++++++++------------------
 1 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/net/pch_gbe/pch_gbe_main.c b/drivers/net/pch_gbe/pch_gbe_main.c
index 5474189..b8b4ba2 100644
--- a/drivers/net/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/pch_gbe/pch_gbe_main.c
@@ -1199,6 +1199,8 @@ static irqreturn_t pch_gbe_intr(int irq, void *data)
 			iowrite32((int_en & ~PCH_GBE_INT_RX_FIFO_ERR),
 				  &hw->reg->INT_EN);
 			pch_gbe_stop_receive(adapter);
+			int_st |= ioread32(&hw->reg->INT_ST);
+			int_st = int_st & ioread32(&hw->reg->INT_EN);
 		}
 	if (int_st & PCH_GBE_INT_RX_DMA_ERR)
 		adapter->stats.intr_rx_dma_err_count++;
@@ -1218,14 +1220,11 @@ static irqreturn_t pch_gbe_intr(int irq, void *data)
 			/* Set Pause packet */
 			pch_gbe_mac_set_pause_packet(hw);
 		}
-		if ((int_en & (PCH_GBE_INT_RX_DMA_CMPLT | PCH_GBE_INT_TX_CMPLT))
-		    == 0) {
-			return IRQ_HANDLED;
-		}
 	}
 
 	/* When request status is Receive interruption */
-	if ((int_st & (PCH_GBE_INT_RX_DMA_CMPLT | PCH_GBE_INT_TX_CMPLT))) {
+	if ((int_st & (PCH_GBE_INT_RX_DMA_CMPLT | PCH_GBE_INT_TX_CMPLT)) ||
+	    (adapter->rx_stop_flag == true)) {
 		if (likely(napi_schedule_prep(&adapter->napi))) {
 			/* Enable only Rx Descriptor empty */
 			atomic_inc(&adapter->irq_sem);
@@ -1385,7 +1384,7 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	struct sk_buff *skb;
 	unsigned int i;
 	unsigned int cleaned_count = 0;
-	bool cleaned = false;
+	bool cleaned = true;
 
 	pr_debug("next_to_clean : %d\n", tx_ring->next_to_clean);
 
@@ -1396,7 +1395,6 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 
 	while ((tx_desc->gbec_status & DSC_INIT16) == 0x0000) {
 		pr_debug("gbec_status:0x%04x\n", tx_desc->gbec_status);
-		cleaned = true;
 		buffer_info = &tx_ring->buffer_info[i];
 		skb = buffer_info->skb;
 
@@ -1439,8 +1437,10 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 		tx_desc = PCH_GBE_TX_DESC(*tx_ring, i);
 
 		/* weight of a sort for tx, to avoid endless transmit cleanup */
-		if (cleaned_count++ == PCH_GBE_TX_WEIGHT)
+		if (cleaned_count++ == PCH_GBE_TX_WEIGHT) {
+			cleaned = false;
 			break;
+		}
 	}
 	pr_debug("called pch_gbe_unmap_and_free_tx_resource() %d count\n",
 		 cleaned_count);
@@ -2168,7 +2168,6 @@ static int pch_gbe_napi_poll(struct napi_struct *napi, int budget)
 {
 	struct pch_gbe_adapter *adapter =
 	    container_of(napi, struct pch_gbe_adapter, napi);
-	struct net_device *netdev = adapter->netdev;
 	int work_done = 0;
 	bool poll_end_flag = false;
 	bool cleaned = false;
@@ -2176,33 +2175,32 @@ static int pch_gbe_napi_poll(struct napi_struct *napi, int budget)
 
 	pr_debug("budget : %d\n", budget);
 
-	/* Keep link state information with original netdev */
-	if (!netif_carrier_ok(netdev)) {
+	pch_gbe_clean_rx(adapter, adapter->rx_ring, &work_done, budget);
+	cleaned = pch_gbe_clean_tx(adapter, adapter->tx_ring);
+
+	if (!cleaned)
+		work_done = budget;
+	/* If no Tx and not enough Rx work done,
+	 * exit the polling mode
+	 */
+	if (work_done < budget)
 		poll_end_flag = true;
-	} else {
-		pch_gbe_clean_rx(adapter, adapter->rx_ring, &work_done, budget);
+
+	if (poll_end_flag) {
+		napi_complete(napi);
+		if (adapter->rx_stop_flag) {
+			adapter->rx_stop_flag = false;
+			pch_gbe_start_receive(&adapter->hw);
+		}
+		pch_gbe_irq_enable(adapter);
+	} else
 		if (adapter->rx_stop_flag) {
 			adapter->rx_stop_flag = false;
 			pch_gbe_start_receive(&adapter->hw);
 			int_en = ioread32(&adapter->hw.reg->INT_EN);
 			iowrite32((int_en | PCH_GBE_INT_RX_FIFO_ERR),
-					&adapter->hw.reg->INT_EN);
+				&adapter->hw.reg->INT_EN);
 		}
-		cleaned = pch_gbe_clean_tx(adapter, adapter->tx_ring);
-
-		if (cleaned)
-			work_done = budget;
-		/* If no Tx and not enough Rx work done,
-		 * exit the polling mode
-		 */
-		if (work_done < budget)
-			poll_end_flag = true;
-	}
-
-	if (poll_end_flag) {
-		napi_complete(napi);
-		pch_gbe_irq_enable(adapter);
-	}
 
 	pr_debug("poll_end_flag : %d  work_done : %d  budget : %d\n",
 		 poll_end_flag, work_done, budget);
-- 
1.7.4.4

^ permalink raw reply related

* Re: [PATCH] tcp: properly release md5sig_pool references
From: Yan, Zheng @ 2011-09-26  8:11 UTC (permalink / raw)
  To: Eric Dumazet, netdev@vger.kernel.org
  Cc: davem@davemloft.net, yoshfuji@linux-ipv6.org
In-Reply-To: <1317018369.2853.16.camel@edumazet-laptop>

On 09/26/2011 02:26 PM, Eric Dumazet wrote:
> Le lundi 26 septembre 2011 à 10:35 +0800, Yan, Zheng a écrit :
>> tcp_v4_md5_do_add() increases use count of md5sig_pool by one for
>> each tcp md5sig peer, so we should call tcp_free_md5sig_pool() for
>> each peer when destroying the socket.
>>
>> Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
> 
> As managing the use count is rather expensive, I think it would be
> better to hold a single reference instead of multiple ones ?
> 
> 

Here is the new patch, thanks.

---
tcp: properly handle md5sig_pool references

tcp_v4_clear_md5_list() assumes that multiple tcp md5sig peers
only hold one reference to md5sig_pool. but tcp_v4_md5_do_add()
increases use count of md5sig_pool for each peer. This patch
makes tcp_v4_md5_do_add() only increases use count for the first
tcp md5sig peer.

Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
---
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c34f015..29ecfa9 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -927,18 +927,20 @@ int tcp_v4_md5_do_add(struct sock *sk, __be32 addr,
 			}
 			sk_nocaps_add(sk, NETIF_F_GSO_MASK);
 		}
-		if (tcp_alloc_md5sig_pool(sk) == NULL) {
+
+		md5sig = tp->md5sig_info;
+		if (md5sig->entries4 == 0 && tcp_alloc_md5sig_pool(sk) == NULL) {
 			kfree(newkey);
 			return -ENOMEM;
 		}
-		md5sig = tp->md5sig_info;
 
 		if (md5sig->alloced4 == md5sig->entries4) {
 			keys = kmalloc((sizeof(*keys) *
 					(md5sig->entries4 + 1)), GFP_ATOMIC);
 			if (!keys) {
 				kfree(newkey);
-				tcp_free_md5sig_pool();
+				if (md5sig->entries4 == 0)
+					tcp_free_md5sig_pool();
 				return -ENOMEM;
 			}
 
@@ -982,6 +984,7 @@ int tcp_v4_md5_do_del(struct sock *sk, __be32 addr)
 				kfree(tp->md5sig_info->keys4);
 				tp->md5sig_info->keys4 = NULL;
 				tp->md5sig_info->alloced4 = 0;
+				tcp_free_md5sig_pool();
 			} else if (tp->md5sig_info->entries4 != i) {
 				/* Need to do some manipulation */
 				memmove(&tp->md5sig_info->keys4[i],
@@ -989,7 +992,6 @@ int tcp_v4_md5_do_del(struct sock *sk, __be32 addr)
 					(tp->md5sig_info->entries4 - i) *
 					 sizeof(struct tcp4_md5sig_key));
 			}
-			tcp_free_md5sig_pool();
 			return 0;
 		}
 	}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3c9fa61..20ac0fc 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -591,7 +591,8 @@ static int tcp_v6_md5_do_add(struct sock *sk, const struct in6_addr *peer,
 			}
 			sk_nocaps_add(sk, NETIF_F_GSO_MASK);
 		}
-		if (tcp_alloc_md5sig_pool(sk) == NULL) {
+		if (tp->md5sig_info->entries6 == 0 &&
+			tcp_alloc_md5sig_pool(sk) == NULL) {
 			kfree(newkey);
 			return -ENOMEM;
 		}
@@ -600,8 +601,9 @@ static int tcp_v6_md5_do_add(struct sock *sk, const struct in6_addr *peer,
 				       (tp->md5sig_info->entries6 + 1)), GFP_ATOMIC);
 
 			if (!keys) {
-				tcp_free_md5sig_pool();
 				kfree(newkey);
+				if (tp->md5sig_info->entries6 == 0)
+					tcp_free_md5sig_pool();
 				return -ENOMEM;
 			}
 
@@ -647,6 +649,7 @@ static int tcp_v6_md5_do_del(struct sock *sk, const struct in6_addr *peer)
 				kfree(tp->md5sig_info->keys6);
 				tp->md5sig_info->keys6 = NULL;
 				tp->md5sig_info->alloced6 = 0;
+				tcp_free_md5sig_pool();
 			} else {
 				/* shrink the database */
 				if (tp->md5sig_info->entries6 != i)
@@ -655,7 +658,6 @@ static int tcp_v6_md5_do_del(struct sock *sk, const struct in6_addr *peer)
 						(tp->md5sig_info->entries6 - i)
 						* sizeof (tp->md5sig_info->keys6[0]));
 			}
-			tcp_free_md5sig_pool();
 			return 0;
 		}
 	}

^ permalink raw reply related

* Re: [PATCH net-next] can/sja1000: add driver for EMS PCMCIA card
From: Markus Plessing @ 2011-09-26  8:15 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4E7CBB04.8070807-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

Am 23.09.2011 18:59, schrieb Oliver Hartkopp:
> This patch adds the driver for the SJA1000 based PCMCIA card 'CPC-Card' from
> EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).
>
> Signed-off-by: Oliver Hartkopp<socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

Acked-by: Markus Plessing <plessing-zsNKPWJ8Pib6hrUXjxyGrA@public.gmane.org>

Thanks Oliver.

> ---
>
> diff -u -r -N a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
> --- a/drivers/net/can/sja1000/Kconfig	2011-09-23 18:02:35.711750820 +0200
> +++ b/drivers/net/can/sja1000/Kconfig	2011-09-23 18:02:28.695751113 +0200
> @@ -29,6 +29,13 @@
>   	  OpenFirmware bindings, e.g. if you have a PowerPC based system
>   	  you may want to enable this option.
>
> +config CAN_EMS_PCMCIA
> +	tristate "EMS CPC-CARD Card"
> +	depends on PCMCIA
> +	---help---
> +	  This driver is for the one or two channel CPC-CARD cards from
> +	  EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).
> +
>   config CAN_EMS_PCI
>   	tristate "EMS CPC-PCI, CPC-PCIe and CPC-104P Card"
>   	depends on PCI
> diff -u -r -N a/drivers/net/can/sja1000/Makefile b/drivers/net/can/sja1000/Makefile
> --- a/drivers/net/can/sja1000/Makefile	2011-09-23 18:02:38.595750534 +0200
> +++ b/drivers/net/can/sja1000/Makefile	2011-09-23 18:02:28.695751113 +0200
> @@ -6,6 +6,7 @@
>   obj-$(CONFIG_CAN_SJA1000_ISA) += sja1000_isa.o
>   obj-$(CONFIG_CAN_SJA1000_PLATFORM) += sja1000_platform.o
>   obj-$(CONFIG_CAN_SJA1000_OF_PLATFORM) += sja1000_of_platform.o
> +obj-$(CONFIG_CAN_EMS_PCMCIA) += ems_pcmcia.o
>   obj-$(CONFIG_CAN_EMS_PCI) += ems_pci.o
>   obj-$(CONFIG_CAN_KVASER_PCI) += kvaser_pci.o
>   obj-$(CONFIG_CAN_PEAK_PCI) += peak_pci.o
> diff -u -r -N a/drivers/net/can/sja1000/ems_pcmcia.c b/drivers/net/can/sja1000/ems_pcmcia.c
> --- a/drivers/net/can/sja1000/ems_pcmcia.c	1970-01-01 01:00:00.000000000 +0100
> +++ b/drivers/net/can/sja1000/ems_pcmcia.c	2011-09-23 18:13:59.227726972 +0200
> @@ -0,0 +1,331 @@
> +/*
> + * Copyright (C) 2008 Sebastian Haas (initial chardev implementation)
> + * Copyright (C) 2010 Markus Plessing<plessing-zsNKPWJ8Pib6hrUXjxyGrA@public.gmane.org>
> + * Rework for mainline by Oliver Hartkopp<socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the version 2 of the GNU General Public License
> + * as published by the Free Software Foundation
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include<linux/kernel.h>
> +#include<linux/module.h>
> +#include<linux/interrupt.h>
> +#include<linux/netdevice.h>
> +#include<linux/delay.h>
> +#include<linux/io.h>
> +#include<pcmcia/cistpl.h>
> +#include<pcmcia/ds.h>
> +#include<linux/can.h>
> +#include<linux/can/dev.h>
> +#include "sja1000.h"
> +
> +#define DRV_NAME "ems_pcmcia"
> +
> +MODULE_AUTHOR("Markus Plessing<plessing-zsNKPWJ8Pib6hrUXjxyGrA@public.gmane.org>");
> +MODULE_DESCRIPTION("Socket-CAN driver for EMS CPC-CARD cards");
> +MODULE_SUPPORTED_DEVICE("EMS CPC-CARD CAN card");
> +MODULE_LICENSE("GPL v2");
> +
> +#define EMS_PCMCIA_MAX_CHAN 2
> +
> +struct ems_pcmcia_card {
> +	int channels;
> +	struct pcmcia_device *pcmcia_dev;
> +	struct net_device *net_dev[EMS_PCMCIA_MAX_CHAN];
> +	void __iomem *base_addr;
> +};
> +
> +#define EMS_PCMCIA_CAN_CLOCK (16000000 / 2)
> +
> +/*
> + * The board configuration is probably following:
> + * RX1 is connected to ground.
> + * TX1 is not connected.
> + * CLKO is not connected.
> + * Setting the OCR register to 0xDA is a good idea.
> + * This means  normal output mode , push-pull and the correct polarity.
> + */
> +#define EMS_PCMCIA_OCR (OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL)
> +
> +/*
> + * In the CDR register, you should set CBP to 1.
> + * You will probably also want to set the clock divider value to 7
> + * (meaning direct oscillator output) because the second SJA1000 chip
> + * is driven by the first one CLKOUT output.
> + */
> +#define EMS_PCMCIA_CDR (CDR_CBP | CDR_CLKOUT_MASK)
> +#define EMS_PCMCIA_MEM_SIZE 4096 /* Size of the remapped io-memory */
> +#define EMS_PCMCIA_CAN_BASE_OFFSET 0x100 /* Offset where controllers starts */
> +#define EMS_PCMCIA_CAN_CTRL_SIZE 0x80 /* Memory size for each controller */
> +
> +#define EMS_CMD_RESET 0x00 /* Perform a reset of the card */
> +#define EMS_CMD_MAP   0x03 /* Map CAN controllers into card' memory */
> +#define EMS_CMD_UMAP  0x02 /* Unmap CAN controllers from card' memory */
> +
> +static struct pcmcia_device_id ems_pcmcia_tbl[] = {
> +	PCMCIA_DEVICE_PROD_ID123("EMS_T_W", "CPC-Card", "V2.0", 0xeab1ea23,
> +				 0xa338573f, 0xe4575800),
> +	PCMCIA_DEVICE_NULL,
> +};
> +
> +MODULE_DEVICE_TABLE(pcmcia, ems_pcmcia_tbl);
> +
> +static u8 ems_pcmcia_read_reg(const struct sja1000_priv *priv, int port)
> +{
> +	return readb(priv->reg_base + port);
> +}
> +
> +static void ems_pcmcia_write_reg(const struct sja1000_priv *priv, int port,
> +				 u8 val)
> +{
> +	writeb(val, priv->reg_base + port);
> +}
> +
> +static irqreturn_t ems_pcmcia_interrupt(int irq, void *dev_id)
> +{
> +	struct ems_pcmcia_card *card = dev_id;
> +	struct net_device *dev;
> +	irqreturn_t retval = IRQ_NONE;
> +	int i, again;
> +
> +	/* Card not present */
> +	if (readw(card->base_addr) != 0xAA55)
> +		return IRQ_HANDLED;
> +
> +	do {
> +		again = 0;
> +
> +		/* Check interrupt for each channel */
> +		for (i = 0; i<  card->channels; i++) {
> +			dev = card->net_dev[i];
> +			if (!dev)
> +				continue;
> +
> +			if (sja1000_interrupt(irq, dev) == IRQ_HANDLED)
> +				again = 1;
> +		}
> +		/* At least one channel handled the interrupt */
> +		if (again)
> +			retval = IRQ_HANDLED;
> +
> +	} while (again);
> +
> +	return retval;
> +}
> +
> +/*
> + * Check if a CAN controller is present at the specified location
> + * by trying to set 'em into the PeliCAN mode
> + */
> +static inline int ems_pcmcia_check_chan(struct sja1000_priv *priv)
> +{
> +	/* Make sure SJA1000 is in reset mode */
> +	ems_pcmcia_write_reg(priv, REG_MOD, 1);
> +	ems_pcmcia_write_reg(priv, REG_CDR, CDR_PELICAN);
> +
> +	/* read reset-values */
> +	if (ems_pcmcia_read_reg(priv, REG_CDR) == CDR_PELICAN)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static void ems_pcmcia_del_card(struct pcmcia_device *pdev)
> +{
> +	struct ems_pcmcia_card *card = pdev->priv;
> +	struct net_device *dev;
> +	int i;
> +
> +	free_irq(pdev->irq, card);
> +
> +	for (i = 0; i<  card->channels; i++) {
> +		dev = card->net_dev[i];
> +		if (!dev)
> +			continue;
> +
> +		printk(KERN_INFO "%s: removing %s on channel #%d\n",
> +		       DRV_NAME, dev->name, i);
> +		unregister_sja1000dev(dev);
> +		free_sja1000dev(dev);
> +	}
> +
> +	writeb(EMS_CMD_UMAP, card->base_addr);
> +	iounmap(card->base_addr);
> +	kfree(card);
> +
> +	pdev->priv = NULL;
> +}
> +
> +/*
> + * Probe PCI device for EMS CAN signature and register each available
> + * CAN channel to SJA1000 Socket-CAN subsystem.
> + */
> +static int __devinit ems_pcmcia_add_card(struct pcmcia_device *pdev,
> +					 unsigned long base)
> +{
> +	struct sja1000_priv *priv;
> +	struct net_device *dev;
> +	struct ems_pcmcia_card *card;
> +	int err, i;
> +
> +	/* Allocating card structures to hold addresses, ... */
> +	card = kzalloc(sizeof(struct ems_pcmcia_card), GFP_KERNEL);
> +	if (!card)
> +		return -ENOMEM;
> +
> +	pdev->priv = card;
> +	card->channels = 0;
> +
> +	card->base_addr = ioremap(base, EMS_PCMCIA_MEM_SIZE);
> +	if (!card->base_addr) {
> +		err = -ENOMEM;
> +		goto failure_cleanup;
> +	}
> +
> +	/* Check for unique EMS CAN signature */
> +	if (readw(card->base_addr) != 0xAA55) {
> +		err = -ENODEV;
> +		goto failure_cleanup;
> +	}
> +
> +	/* Request board reset */
> +	writeb(EMS_CMD_RESET, card->base_addr);
> +
> +	/* Make sure CAN controllers are mapped into card's memory space */
> +	writeb(EMS_CMD_MAP, card->base_addr);
> +
> +	/* Detect available channels */
> +	for (i = 0; i<  EMS_PCMCIA_MAX_CHAN; i++) {
> +		dev = alloc_sja1000dev(0);
> +		if (!dev) {
> +			err = -ENOMEM;
> +			goto failure_cleanup;
> +		}
> +
> +		card->net_dev[i] = dev;
> +		priv = netdev_priv(dev);
> +		priv->priv = card;
> +		SET_NETDEV_DEV(dev,&pdev->dev);
> +
> +		priv->irq_flags = IRQF_SHARED;
> +		dev->irq = pdev->irq;
> +		priv->reg_base = card->base_addr + EMS_PCMCIA_CAN_BASE_OFFSET +
> +			(i * EMS_PCMCIA_CAN_CTRL_SIZE);
> +
> +		/* Check if channel is present */
> +		if (ems_pcmcia_check_chan(priv)) {
> +			priv->read_reg  = ems_pcmcia_read_reg;
> +			priv->write_reg = ems_pcmcia_write_reg;
> +			priv->can.clock.freq = EMS_PCMCIA_CAN_CLOCK;
> +			priv->ocr = EMS_PCMCIA_OCR;
> +			priv->cdr = EMS_PCMCIA_CDR;
> +			priv->flags |= SJA1000_CUSTOM_IRQ_HANDLER;
> +
> +			/* Register SJA1000 device */
> +			err = register_sja1000dev(dev);
> +			if (err) {
> +				free_sja1000dev(dev);
> +				goto failure_cleanup;
> +			}
> +
> +			card->channels++;
> +
> +			printk(KERN_INFO "%s: registered %s on channel "
> +			       "#%d at 0x%p, irq %d\n", DRV_NAME, dev->name,
> +			       i, priv->reg_base, dev->irq);
> +		} else
> +			free_sja1000dev(dev);
> +	}
> +
> +	err = request_irq(dev->irq,&ems_pcmcia_interrupt, IRQF_SHARED,
> +			  DRV_NAME, card);
> +	if (!err)
> +		return 0;
> +
> +failure_cleanup:
> +	ems_pcmcia_del_card(pdev);
> +	return err;
> +}
> +
> +/*
> + * Setup PCMCIA socket and probe for EMS CPC-CARD
> + */
> +static int __devinit ems_pcmcia_probe(struct pcmcia_device *dev)
> +{
> +	int csval;
> +
> +	/* General socket configuration */
> +	dev->config_flags |= CONF_ENABLE_IRQ;
> +	dev->config_index = 1;
> +	dev->config_regs = PRESENT_OPTION;
> +
> +	/* The io structure describes IO port mapping */
> +	dev->resource[0]->end = 16;
> +	dev->resource[0]->flags |= IO_DATA_PATH_WIDTH_8;
> +	dev->resource[1]->end = 16;
> +	dev->resource[1]->flags |= IO_DATA_PATH_WIDTH_16;
> +	dev->io_lines = 5;
> +
> +	/* Allocate a memory window */
> +	dev->resource[2]->flags =
> +		(WIN_DATA_WIDTH_8 | WIN_MEMORY_TYPE_CM | WIN_ENABLE);
> +	dev->resource[2]->start = dev->resource[2]->end = 0;
> +
> +	csval = pcmcia_request_window(dev, dev->resource[2], 0);
> +	if (csval) {
> +		dev_err(&dev->dev, "pcmcia_request_window failed (err=%d)\n",
> +			csval);
> +		return 0;
> +	}
> +
> +	csval = pcmcia_map_mem_page(dev, dev->resource[2], dev->config_base);
> +	if (csval) {
> +		dev_err(&dev->dev, "pcmcia_map_mem_page failed (err=%d)\n",
> +			csval);
> +		return 0;
> +	}
> +
> +	csval = pcmcia_enable_device(dev);
> +	if (csval) {
> +		dev_err(&dev->dev, "pcmcia_enable_device failed (err=%d)\n",
> +			csval);
> +		return 0;
> +	}
> +
> +	ems_pcmcia_add_card(dev, dev->resource[2]->start);
> +	return 0;
> +}
> +
> +/*
> + * Release claimed resources
> + */
> +static void ems_pcmcia_remove(struct pcmcia_device *dev)
> +{
> +	ems_pcmcia_del_card(dev);
> +	pcmcia_disable_device(dev);
> +}
> +
> +static struct pcmcia_driver ems_pcmcia_driver = {
> +	.name = DRV_NAME,
> +	.probe = ems_pcmcia_probe,
> +	.remove = ems_pcmcia_remove,
> +	.id_table = ems_pcmcia_tbl,
> +};
> +
> +static int __init ems_pcmcia_init(void)
> +{
> +	return pcmcia_register_driver(&ems_pcmcia_driver);
> +}
> +module_init(ems_pcmcia_init);
> +
> +static void __exit ems_pcmcia_exit(void)
> +{
> +	pcmcia_unregister_driver(&ems_pcmcia_driver);
> +}
> +module_exit(ems_pcmcia_exit);

^ permalink raw reply

* Re: [PATCH] tcp: ECN blackhole should not force quickack mode
From: Eric Dumazet @ 2011-09-26  8:26 UTC (permalink / raw)
  To: jhs
  Cc: David Miller, netdev, Jerry Chu, Ilpo Järvinen, Jim Gettys,
	Dave Taht
In-Reply-To: <1316999597.1865.37.camel@mojatatu>

Le dimanche 25 septembre 2011 à 21:13 -0400, jamal a écrit :
> On Sun, 2011-09-25 at 21:07 -0400, jamal wrote:
> 
> > In regards to the 'linux funny extension' - this was a brilliant idea
> > in my opinion back then from Alexey; lots of discussions happened but
> > I cant remember if it made it in some RFC or not (I will try to search
> > some old archives).
> 
> Wasnt hard:
> https://tools.ietf.org/id/draft-ietf-tsvwg-tcp-ecn-00.txt
> It doesnt seem to have made it to RFC and i cant remember why.
> [We dont wanna bring Sally out of retirement but we can ask KK if
> it bugs you;->]

Thanks !

This refers to additions to RFC 2481 : This was refined by RFC 3168, and
the retransmitted TCP packets requirement is part of the final RFC :

6.1.5.  Retransmitted TCP packets

   This document specifies ECN-capable TCP implementations MUST NOT set
   either ECT codepoint (ECT(0) or ECT(1)) in the IP header for
   retransmitted data packets, ...

Followed by a very long description :)

BTW, the ECN+ proposal (RFC 5562 :  Adding Explicit Congestion
Notification (ECN) Capability to TCP's SYN/ACK Packets) would allow the
client (receiving SYNACK message with ECT flags) to set the TCP_ECN_SEEN
I added in my patch, allowing even the first (retransmitted) data packet
to trigger quickack mode.

^ permalink raw reply

* Re: [PATCH 0/4] IPVS: sysctl Documentation and Various Clean-Ups
From: Simon Horman @ 2011-09-26  8:33 UTC (permalink / raw)
  To: Hans Schillstrom
  Cc: lvs-devel, netdev, netfilter-devel, netfilter, Wensong Zhang,
	Julian Anastasov, Patrick McHardy
In-Reply-To: <201109260843.18257.hans@schillstrom.com>

On Mon, Sep 26, 2011 at 08:43:17AM +0200, Hans Schillstrom wrote:
> Hello Simon
> 
> On Monday, September 26, 2011 03:23:49 Simon Horman wrote:
> > Hi,
> > 
> > patch one of this series adds some missing entries to the sysctl
> > documentation. Julian, I would appreciate a review of this change.
> > 
> > The remaining three patches are minor code clean-ups flagged
> > by gcc 4.6.1.
> > 
> > My intention for this post is to solicit review.
> > I intend to post a pull request separately at a later date.
> > 
> 
> Your patches looks fine to me,
> you can have mine ack as well
> 
> Acked-by Hans Schillstrom <hans@schillstrom.com>
> 
> I have two small pending patches,  Deadlock and lockdep fixes. 
> Hopefully the patches will be ready this week. The deadlock is a tricky bastard to fix. 

Ok. I will try and send a pull request to Patrick before then,
but feel free to send the patches any time. Do you expect
they will be -stable material?

^ permalink raw reply

* Re: [PATCH 0/4] IPVS: sysctl Documentation and Various Clean-Ups
From: Simon Horman @ 2011-09-26  8:37 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: lvs-devel, netdev, netfilter-devel, netfilter, Wensong Zhang,
	Patrick McHardy
In-Reply-To: <alpine.LFD.2.00.1109260829190.1530@ja.ssi.bg>

On Mon, Sep 26, 2011 at 08:39:02AM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 26 Sep 2011, Simon Horman wrote:
> 
> > Hi,
> > 
> > patch one of this series adds some missing entries to the sysctl
> > documentation. Julian, I would appreciate a review of this change.
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>
> 
> 	for all changes, with one minor problem in patch 1
> for sync_version: 'sh[uo]ld be used [Use this ]when'

Thanks, fixed.

^ permalink raw reply

* Re: [PATCH 2/2] net: Fix potential memory leak
From: Huajun Li @ 2011-09-26  8:43 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Huajun Li
In-Reply-To: <1317017353.2853.13.camel@edumazet-laptop>

Eric, thanks for your comment.

2011/9/26 Eric Dumazet <eric.dumazet@gmail.com>:
> Le samedi 24 septembre 2011 à 23:57 +0800, Huajun Li a écrit :
>> While preparing flow caches, once fail may cause potential memory leak , fix it.
>>
>> Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
>> ---
>>  net/core/flow.c |   19 ++++++++++++++++++-
>>  1 files changed, 18 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/core/flow.c b/net/core/flow.c
>> index ba3e617..2dcaa03 100644
>> --- a/net/core/flow.c
>> +++ b/net/core/flow.c
>> @@ -420,7 +420,7 @@ static int __init flow_cache_init(struct flow_cache *fc)
>>
>>       for_each_online_cpu(i) {
>>               if (flow_cache_cpu_prepare(fc, i))
>> -                     return -ENOMEM;
>> +                     goto err;
>>       }
>>       fc->hotcpu_notifier = (struct notifier_block){
>>               .notifier_call = flow_cache_cpu,
>> @@ -433,6 +433,23 @@ static int __init flow_cache_init(struct flow_cache *fc)
>>       add_timer(&fc->rnd_timer);
>>
>>       return 0;
>> +err:
>> +     if (fc->percpu) {
>> +             free_percpu(fc->percpu);
>> +             fc->percpu = NULL;
>> +     }
>> +
>> +     /*
>> +      * Check each possible CPUs rather than online ones because they may be
>> +      * offline before the notifier is registered.
>> +      */
>
> Please remove this comment.
>

Sure.

>
>> +     for_each_possible_cpu(i) {
>> +             struct flow_cache_percpu *fcp = per_cpu_ptr(fc->percpu, i);
>> +             kfree(fcp->hash_table);
>> +             fcp->hash_table = NULL;
>> +     }
>
> You access fc->percpu after freeing it...
>

Yes, need change the order to free memory.

>> +
>> +     return -ENOMEM;
>>  }
>>
>>  static int __init flow_cache_init_global(void)
>
> Previous to 2.6.37 (commit 83b6b1f5d134), a memory allocation at this
> stage was panicing the box, so no worry about mem leak :)
>
> Now I wonder if a proper patch would not print a nice message in
> flow_cache_init_global() if flow_cache_init() returns an error, instead
> of silently panicing or something worse...
>

There prints err msg in flow_cache_cpu_prepare(L369) if fails to
allocate memory. Do you mean it should give more detail error info,
right ?

> Before submitting a new patch, could you test this case (injecting a
> memalloc error in flow_cache_cpu_prepare() for example.
>

Will test it further if new patch comes.  ;)

^ permalink raw reply

* Re: [PATCH 1/2] net: Disable false positive memory leak report
From: Huajun Li @ 2011-09-26  8:42 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Huajun Li
In-Reply-To: <1317014958.2853.0.camel@edumazet-laptop>

Eric, thanks for your response. :)

2011/9/26 Eric Dumazet <eric.dumazet@gmail.com>:
> Le samedi 24 septembre 2011 à 23:57 +0800, Huajun Li a écrit :
>> Memory leak detector reports following false positive memory leak, the
>> patch disables it.
>>
>> unreferenced object 0xffff880073a70000 (size 8192):
>>   comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
>>   hex dump (first 32 bytes):
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<ffffffff8124db64>] create_object+0x144/0x360
>>     [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
>>     [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
>>     [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
>>     [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
>>     [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
>>     [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
>>     [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
>>     [<ffffffffffffffff>] 0xffffffffffffffff
>> unreferenced object 0xffff880073a74290 (size 8192):
>>   comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
>>   hex dump (first 32 bytes):
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<ffffffff8124db64>] create_object+0x144/0x360
>>     [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
>>     [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
>>     [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
>>     [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
>>     [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
>>     [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
>>     [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
>>     [<ffffffffffffffff>] 0xffffffffffffffff
>>
>>
>> Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
>> ---
>>  net/core/flow.c |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/core/flow.c b/net/core/flow.c
>> index 555a456..ba3e617 100644
>> --- a/net/core/flow.c
>> +++ b/net/core/flow.c
>> @@ -365,6 +365,13 @@ static int __cpuinit
>> flow_cache_cpu_prepare(struct flow_cache *fc, int cpu)
>>
>>       if (!fcp->hash_table) {
>>               fcp->hash_table = kzalloc_node(sz, GFP_KERNEL, cpu_to_node(cpu));
>> +             /*
>> +              * Avoid a kmemleak false positive. The pointer to this block
>> +              * is refferenced by per-CPU varaible, here just mark it as not
>> +              * being a leak.
>> +              */
>> +             kmemleak_not_leak(fcp->hash_table);
>> +
>>               if (!fcp->hash_table) {
>>                       pr_err("NET: failed to allocate flow cache sz %zu\n", sz);
>>                       return -ENOMEM;
>
> This makes no sense to me.
>
> per-cpu variables are taken into account by kmemleak.
>

Yes, I think so.

> If not, you should report this problem to kmemleak maintainer.
>

To deal with false positives/negatives, in kmemleak document, it says:
...
The false positives are objects wrongly reported as being memory leaks
(orphan). For objects known not to be leaks, kmemleak provides the
kmemleak_not_leak function. The kmemleak_ignore could also be used if
the memory block is known not to contain other pointers and it will no
longer be scanned.
...

For some scenarios, _maybe_ it's hard for kmemleak to give report
exactly, and I think it is known issue for the maintainers, so hope
there will be a powerful version soon. :)
However, before the new version of kmemleak(if there is new version)
comes, is it possible to disable this false positive report as other
components? You know, other guys maybe also check the code once they
meet the report again just like me.

^ permalink raw reply

* [GIT PULL net-next] IPVS
From: Simon Horman @ 2011-09-26  9:05 UTC (permalink / raw)
  To: lvs-devel, netdev, netfilter-devel, netfilter
  Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
	Pablo Neira Ayuso, David S. Miller

Hi,

with all the excitement of kernel.org being offline and a bunch of trees
likewise being offline I am a little unsure who should take this pull
request which is based on the current net-next tree. But I guess it should
be Patrick, Pablo or Dave.

The following changes since commit 7777de9af54a1402c79bf7663b38ff5ba308dd45:

  qlcnic: Change CDRP function (2011-09-23 14:23:59 -0400)

are available in the git repository at:
  git://github.com/horms/ipvs-next.git master

Krzysztof Wilczynski (1):
      ipvs: Expose ip_vs_ftp module parameters via sysfs.

Simon Horman (4):
      IPVS: Add documentation for new sysctl entries
      IPVS: Remove unused parameter from ip_vs_confirm_conntrack()
      IPVS: Remove unused return value of protocol state transitions
      IPVS: Removed unused variables

 Documentation/networking/ipvs-sysctl.txt |   52 +++++++++++++++++++++++++++++-
 include/net/ip_vs.h                      |   11 +++---
 net/netfilter/ipvs/ip_vs_core.c          |   13 +++----
 net/netfilter/ipvs/ip_vs_ctl.c           |    4 --
 net/netfilter/ipvs/ip_vs_ftp.c           |    5 ++-
 net/netfilter/ipvs/ip_vs_nfct.c          |    2 +-
 net/netfilter/ipvs/ip_vs_proto_sctp.c    |   14 ++------
 net/netfilter/ipvs/ip_vs_proto_tcp.c     |    6 +--
 net/netfilter/ipvs/ip_vs_proto_udp.c     |    5 +--
 net/netfilter/ipvs/ip_vs_xmit.c          |    2 +-
 10 files changed, 75 insertions(+), 39 deletions(-)

^ permalink raw reply

* [PATCH 2/5] IPVS: Add documentation for new sysctl entries
From: Simon Horman @ 2011-09-26  9:05 UTC (permalink / raw)
  To: lvs-devel, netdev, netfilter-devel, netfilter
  Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
	Pablo Neira Ayuso, David S. Miller, Simon Horman
In-Reply-To: <1317027938-22053-1-git-send-email-horms@verge.net.au>

Add missing documentation for conntrack, snat_reroute and sync_version.

Also fix up a typo, IPVS_DEBUG should be IP_VS_DEBUG.

Acked-by: Julian Anastasov <ja@ssi.bg>
Acked-by Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 Documentation/networking/ipvs-sysctl.txt |   52 +++++++++++++++++++++++++++++-
 1 files changed, 51 insertions(+), 1 deletions(-)

diff --git a/Documentation/networking/ipvs-sysctl.txt b/Documentation/networking/ipvs-sysctl.txt
index 4ccdbca..1dcdd49 100644
--- a/Documentation/networking/ipvs-sysctl.txt
+++ b/Documentation/networking/ipvs-sysctl.txt
@@ -15,6 +15,23 @@ amemthresh - INTEGER
         enabled and the variable is automatically set to 2, otherwise
         the strategy is disabled and the variable is  set  to 1.
 
+conntrack - BOOLEAN
+	0 - disabled (default)
+	not 0 - enabled
+
+	If set, maintain connection tracking entries for
+	connections handled by IPVS.
+
+	This should be enabled if connections handled by IPVS are to be
+	also handled by stateful firewall rules. That is, iptables rules
+	that make use of connection tracking.  It is a performance
+	optimisation to disable this setting otherwise.
+
+	Connections handled by the IPVS FTP application module
+	will have connection tracking entries regardless of this setting.
+
+	Only available when IPVS is compiled with the CONFIG_IP_VS_NFCT
+
 cache_bypass - BOOLEAN
         0 - disabled (default)
         not 0 - enabled
@@ -39,7 +56,7 @@ debug_level - INTEGER
 	11         - IPVS packet handling (ip_vs_in/ip_vs_out)
 	12 or more - packet traversal
 
-	Only available when IPVS is compiled with the CONFIG_IPVS_DEBUG
+	Only available when IPVS is compiled with the CONFIG_IP_VS_DEBUG
 
 	Higher debugging levels include the messages for lower debugging
 	levels, so setting debug level 2, includes level 0, 1 and 2
@@ -141,3 +158,36 @@ sync_threshold - INTEGER
         synchronized, every time the number of its incoming packets
         modulus 50 equals the threshold. The range of the threshold is
         from 0 to 49.
+
+snat_reroute - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	If enabled, recalculate the route of SNATed packets from
+	realservers so that they are routed as if they originate from the
+	director. Otherwise they are routed as if they are forwarded by the
+	director.
+
+	If policy routing is in effect then it is possible that the route
+	of a packet originating from a director is routed differently to a
+	packet being forwarded by the director.
+
+	If policy routing is not in effect then the recalculated route will
+	always be the same as the original route so it is an optimisation
+	to disable snat_reroute and avoid the recalculation.
+
+sync_version - INTEGER
+	default 1
+
+	The version of the synchronisation protocol used when sending
+	synchronisation messages.
+
+	0 selects the original synchronisation protocol (version 0). This
+	should be used when sending synchronisation messages to a legacy
+	system that only understands the original synchronisation protocol.
+
+	1 selects the current synchronisation protocol (version 1). This
+	should be used where possible.
+
+	Kernels with this sync_version entry are able to receive messages
+	of both version 1 and version 2 of the synchronisation protocol.
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH 4/5] IPVS: Remove unused return value of protocol state transitions
From: Simon Horman @ 2011-09-26  9:05 UTC (permalink / raw)
  To: lvs-devel, netdev, netfilter-devel, netfilter
  Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
	Pablo Neira Ayuso, David S. Miller, Simon Horman
In-Reply-To: <1317027938-22053-1-git-send-email-horms@verge.net.au>

Acked-by: Julian Anastasov <ja@ssi.bg>
Acked-by Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 include/net/ip_vs.h                   |    6 +++---
 net/netfilter/ipvs/ip_vs_core.c       |   13 ++++++-------
 net/netfilter/ipvs/ip_vs_proto_sctp.c |   14 ++++----------
 net/netfilter/ipvs/ip_vs_proto_tcp.c  |    6 ++----
 net/netfilter/ipvs/ip_vs_proto_udp.c  |    5 ++---
 5 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index a5f0f6b..139784e 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -425,9 +425,9 @@ struct ip_vs_protocol {
 
 	const char *(*state_name)(int state);
 
-	int (*state_transition)(struct ip_vs_conn *cp, int direction,
-				const struct sk_buff *skb,
-				struct ip_vs_proto_data *pd);
+	void (*state_transition)(struct ip_vs_conn *cp, int direction,
+				 const struct sk_buff *skb,
+				 struct ip_vs_proto_data *pd);
 
 	int (*register_app)(struct net *net, struct ip_vs_app *inc);
 
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 4f77bb1..00ea1ad 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -188,14 +188,13 @@ ip_vs_conn_stats(struct ip_vs_conn *cp, struct ip_vs_service *svc)
 }
 
 
-static inline int
+static inline void
 ip_vs_set_state(struct ip_vs_conn *cp, int direction,
 		const struct sk_buff *skb,
 		struct ip_vs_proto_data *pd)
 {
-	if (unlikely(!pd->pp->state_transition))
-		return 0;
-	return pd->pp->state_transition(cp, direction, skb, pd);
+	if (likely(pd->pp->state_transition))
+		pd->pp->state_transition(cp, direction, skb, pd);
 }
 
 static inline int
@@ -557,7 +556,7 @@ int ip_vs_leave(struct ip_vs_service *svc, struct sk_buff *skb,
 		ip_vs_in_stats(cp, skb);
 
 		/* set state */
-		cs = ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
+		ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
 
 		/* transmit the first SYN packet */
 		ret = cp->packet_xmit(skb, cp, pd->pp);
@@ -1490,7 +1489,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
 	struct ip_vs_protocol *pp;
 	struct ip_vs_proto_data *pd;
 	struct ip_vs_conn *cp;
-	int ret, restart, pkts;
+	int ret, pkts;
 	struct netns_ipvs *ipvs;
 
 	/* Already marked as IPVS request or reply? */
@@ -1591,7 +1590,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
 	}
 
 	ip_vs_in_stats(cp, skb);
-	restart = ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
+	ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
 	if (cp->packet_xmit)
 		ret = cp->packet_xmit(skb, cp, pp);
 		/* do not touch skb anymore */
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index d12ed53..1fbf7a2 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -906,7 +906,7 @@ static const char *sctp_state_name(int state)
 	return "?";
 }
 
-static inline int
+static inline void
 set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
 		int direction, const struct sk_buff *skb)
 {
@@ -924,7 +924,7 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
 	sch = skb_header_pointer(skb, ihl + sizeof(sctp_sctphdr_t),
 				sizeof(_sctpch), &_sctpch);
 	if (sch == NULL)
-		return 0;
+		return;
 
 	chunk_type = sch->type;
 	/*
@@ -993,21 +993,15 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
 		cp->timeout = pd->timeout_table[cp->state = next_state];
 	else	/* What to do ? */
 		cp->timeout = sctp_timeouts[cp->state = next_state];
-
-	return 1;
 }
 
-static int
+static void
 sctp_state_transition(struct ip_vs_conn *cp, int direction,
 		const struct sk_buff *skb, struct ip_vs_proto_data *pd)
 {
-	int ret = 0;
-
 	spin_lock(&cp->lock);
-	ret = set_sctp_state(pd, cp, direction, skb);
+	set_sctp_state(pd, cp, direction, skb);
 	spin_unlock(&cp->lock);
-
-	return ret;
 }
 
 static inline __u16 sctp_app_hashkey(__be16 port)
diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
index c0cc341..ef8641f 100644
--- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
@@ -546,7 +546,7 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
 /*
  *	Handle state transitions
  */
-static int
+static void
 tcp_state_transition(struct ip_vs_conn *cp, int direction,
 		     const struct sk_buff *skb,
 		     struct ip_vs_proto_data *pd)
@@ -561,13 +561,11 @@ tcp_state_transition(struct ip_vs_conn *cp, int direction,
 
 	th = skb_header_pointer(skb, ihl, sizeof(_tcph), &_tcph);
 	if (th == NULL)
-		return 0;
+		return;
 
 	spin_lock(&cp->lock);
 	set_tcp_state(pd, cp, direction, th);
 	spin_unlock(&cp->lock);
-
-	return 1;
 }
 
 static inline __u16 tcp_app_hashkey(__be16 port)
diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
index f1282cb..f4b7262 100644
--- a/net/netfilter/ipvs/ip_vs_proto_udp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
@@ -454,18 +454,17 @@ static const char * udp_state_name(int state)
 	return udp_state_name_table[state] ? udp_state_name_table[state] : "?";
 }
 
-static int
+static void
 udp_state_transition(struct ip_vs_conn *cp, int direction,
 		     const struct sk_buff *skb,
 		     struct ip_vs_proto_data *pd)
 {
 	if (unlikely(!pd)) {
 		pr_err("UDP no ns data\n");
-		return 0;
+		return;
 	}
 
 	cp->timeout = pd->timeout_table[IP_VS_UDP_S_NORMAL];
-	return 1;
 }
 
 static void __udp_init(struct net *net, struct ip_vs_proto_data *pd)
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH 5/5] IPVS: Removed unused variables
From: Simon Horman @ 2011-09-26  9:05 UTC (permalink / raw)
  To: lvs-devel, netdev, netfilter-devel, netfilter
  Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
	Pablo Neira Ayuso, David S. Miller, Simon Horman
In-Reply-To: <1317027938-22053-1-git-send-email-horms@verge.net.au>

ipvs is not used in ip_vs_genl_set_cmd() or ip_vs_genl_get_cmd()

Acked-by: Julian Anastasov <ja@ssi.bg>
Acked-by Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_ctl.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 2b771dc..8f8aa30 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3279,10 +3279,8 @@ static int ip_vs_genl_set_cmd(struct sk_buff *skb, struct genl_info *info)
 	int ret = 0, cmd;
 	int need_full_svc = 0, need_full_dest = 0;
 	struct net *net;
-	struct netns_ipvs *ipvs;
 
 	net = skb_sknet(skb);
-	ipvs = net_ipvs(net);
 	cmd = info->genlhdr->cmd;
 
 	mutex_lock(&__ip_vs_mutex);
@@ -3392,10 +3390,8 @@ static int ip_vs_genl_get_cmd(struct sk_buff *skb, struct genl_info *info)
 	void *reply;
 	int ret, cmd, reply_cmd;
 	struct net *net;
-	struct netns_ipvs *ipvs;
 
 	net = skb_sknet(skb);
-	ipvs = net_ipvs(net);
 	cmd = info->genlhdr->cmd;
 
 	if (cmd == IPVS_CMD_GET_SERVICE)
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH 1/5] ipvs: Expose ip_vs_ftp module parameters via sysfs.
From: Simon Horman @ 2011-09-26  9:05 UTC (permalink / raw)
  To: lvs-devel, netdev, netfilter-devel, netfilter
  Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
	Pablo Neira Ayuso, David S. Miller, Krzysztof Wilczynski,
	Simon Horman
In-Reply-To: <1317027938-22053-1-git-send-email-horms@verge.net.au>

From: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>

This is to expose "ports" parameter via sysfs so it can be read
at any time in order to determine what port or ports were passed
to the module at the point when it was loaded.

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_ftp.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 4490a32..538d74e 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -52,8 +52,9 @@
  * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper
  * First port is set to the default port.
  */
+static unsigned int ports_count = 1;
 static unsigned short ports[IP_VS_APP_MAX_PORTS] = {21, 0};
-module_param_array(ports, ushort, NULL, 0);
+module_param_array(ports, ushort, &ports_count, 0444);
 MODULE_PARM_DESC(ports, "Ports to monitor for FTP control commands");
 
 
@@ -449,7 +450,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
 	if (ret)
 		goto err_exit;
 
-	for (i=0; i<IP_VS_APP_MAX_PORTS; i++) {
+	for (i = 0; i < ports_count; i++) {
 		if (!ports[i])
 			continue;
 		ret = register_ip_vs_app_inc(net, app, app->protocol, ports[i]);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 3/5] IPVS: Remove unused parameter from ip_vs_confirm_conntrack()
From: Simon Horman @ 2011-09-26  9:05 UTC (permalink / raw)
  To: lvs-devel, netdev, netfilter-devel, netfilter
  Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
	Pablo Neira Ayuso, David S. Miller, Simon Horman
In-Reply-To: <1317027938-22053-1-git-send-email-horms@verge.net.au>

Acked-by: Julian Anastasov <ja@ssi.bg>
Acked-by Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 include/net/ip_vs.h             |    5 ++---
 net/netfilter/ipvs/ip_vs_nfct.c |    2 +-
 net/netfilter/ipvs/ip_vs_xmit.c |    2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 1aaf915..a5f0f6b 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1377,7 +1377,7 @@ static inline int ip_vs_conntrack_enabled(struct netns_ipvs *ipvs)
 
 extern void ip_vs_update_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp,
 				   int outin);
-extern int ip_vs_confirm_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp);
+extern int ip_vs_confirm_conntrack(struct sk_buff *skb);
 extern void ip_vs_nfct_expect_related(struct sk_buff *skb, struct nf_conn *ct,
 				      struct ip_vs_conn *cp, u_int8_t proto,
 				      const __be16 port, int from_rs);
@@ -1395,8 +1395,7 @@ static inline void ip_vs_update_conntrack(struct sk_buff *skb,
 {
 }
 
-static inline int ip_vs_confirm_conntrack(struct sk_buff *skb,
-					  struct ip_vs_conn *cp)
+static inline int ip_vs_confirm_conntrack(struct sk_buff *skb);
 {
 	return NF_ACCEPT;
 }
diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
index f454c80..022e77e 100644
--- a/net/netfilter/ipvs/ip_vs_nfct.c
+++ b/net/netfilter/ipvs/ip_vs_nfct.c
@@ -127,7 +127,7 @@ ip_vs_update_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp, int outin)
 	nf_conntrack_alter_reply(ct, &new_tuple);
 }
 
-int ip_vs_confirm_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp)
+int ip_vs_confirm_conntrack(struct sk_buff *skb)
 {
 	return nf_conntrack_confirm(skb);
 }
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index ee319a4..aa2d720 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -339,7 +339,7 @@ ip_vs_dst_reset(struct ip_vs_dest *dest)
 								\
 	(skb)->ipvs_property = 1;				\
 	if (unlikely((cp)->flags & IP_VS_CONN_F_NFCT))		\
-		__ret = ip_vs_confirm_conntrack(skb, cp);	\
+		__ret = ip_vs_confirm_conntrack(skb);		\
 	if (__ret == NF_ACCEPT) {				\
 		nf_reset(skb);					\
 		skb_forward_csum(skb);				\
-- 
1.7.5.4

^ permalink raw reply related

* [ANNOUNCE] ipvs and ipvs-next trees on github
From: Simon Horman @ 2011-09-26  9:10 UTC (permalink / raw)
  To: lvs-devel, netdev, netfilter-devel, netfilter

Hi,

Like many others I have put some trees up on github to see me
through until kernel.org becomes operation again.

Please base any critical bug fixes for the current rc kernel on
git://github.com/horms/ipvs.git

Please base any other changes on
git://github.com/horms/ipvs-next.git

These trees are currently based on David Miller's net and net-next
trees respectively and follow the same rules for accepting changes
as those trees.


^ permalink raw reply

* Re: [PATCH 1/2] net: Disable false positive memory leak report
From: Eric Dumazet @ 2011-09-26  9:46 UTC (permalink / raw)
  To: Huajun Li; +Cc: David Miller, netdev
In-Reply-To: <CA+v9cxZk_bsxrnn8zS-viH-0Wozz-T0XiFB0AE1fOOKqjcQ2ZQ@mail.gmail.com>

Le lundi 26 septembre 2011 à 16:42 +0800, Huajun Li a écrit :
> Eric, thanks for your response. :)
> 
> 2011/9/26 Eric Dumazet <eric.dumazet@gmail.com>:

> > This makes no sense to me.
> >
> > per-cpu variables are taken into account by kmemleak.
> >
> 
> Yes, I think so.
> 
> > If not, you should report this problem to kmemleak maintainer.
> >
> 
> To deal with false positives/negatives, in kmemleak document, it says:
> ...
> The false positives are objects wrongly reported as being memory leaks
> (orphan). For objects known not to be leaks, kmemleak provides the
> kmemleak_not_leak function. The kmemleak_ignore could also be used if
> the memory block is known not to contain other pointers and it will no
> longer be scanned.
> ...
> 

But this interface should be only used in very specific situations, not
for transient kmemleak reports.

(When the pointer to allocated block is not stored in memory, or only
stored in the object itself : A detached area of memory)

AFAIK, its almost never used :

# git grep kmemleak_not_leak
Documentation/kmemleak.txt:kmemleak_not_leak     - mark an object as not a leak
Documentation/kmemleak.txt:kmemleak_not_leak function. The kmemleak_ignore could also be used if
arch/sparc/kernel/irq_64.c:     kmemleak_not_leak(bucket);
fs/block_dev.c: kmemleak_not_leak(bd_mnt);
fs/nfs/dir.c:   kmemleak_not_leak(string->name);
include/linux/kmemleak.h:extern void kmemleak_not_leak(const void *ptr) __ref;
include/linux/kmemleak.h:static inline void kmemleak_not_leak(const void *ptr)
kernel/module.c:        kmemleak_not_leak(ptr);
mm/kmemleak.c: * kmemleak_not_leak - mark an allocated object as false positive
mm/kmemleak.c:void __ref kmemleak_not_leak(const void *ptr)
mm/kmemleak.c:EXPORT_SYMBOL(kmemleak_not_leak);
mm/kmemleak.c:                  kmemleak_not_leak(log->ptr);
mm/page_cgroup.c:       kmemleak_not_leak(base);


percpu variables are not specific these days, and kmemleak definitely
knows about them.

> For some scenarios, _maybe_ it's hard for kmemleak to give report
> exactly, and I think it is known issue for the maintainers, so hope
> there will be a powerful version soon. :)
> However, before the new version of kmemleak(if there is new version)
> comes, is it possible to disable this false positive report as other
> components? You know, other guys maybe also check the code once they
> meet the report again just like me.

We are not going to add patches to work around a core (kmemleak) issue,
especially without kmemleak maintainer feedback.

^ permalink raw reply

* intel 82599 multi-port performance
From: J.Hwan Kim @ 2011-09-26 10:26 UTC (permalink / raw)
  To: netdev

Hi, everyone

Now, I'm testing a network card including intel 82599.
In our experiment, with the driver modified with ixgbe and multi-port 
enabled,
rx performance of each port with 10Gbps of 64bytes frame is
a half than when only 1 port is used.
The pcie of our server is GEN2 (5x X 8).

Is the result reasonable?
When multi-ports are enabled and 10G stream is inserted to each port,
the maximum performance of each port is a half in our experiment.

Do you think it is a problem of our modified driver or the performance
bottleneck of 82599?
Now I cannot understand our experiment result.
Please give me an advice.

Thanks in advance.

Best Regards,
J.Hwan Kim

^ permalink raw reply

* Re: [PATCH v3 1/7] Basic kernel memory functionality for the Memory Controller
From: KAMEZAWA Hiroyuki @ 2011-09-26 10:34 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill
In-Reply-To: <1316393805-3005-2-git-send-email-glommer@parallels.com>

On Sun, 18 Sep 2011 21:56:39 -0300
Glauber Costa <glommer@parallels.com> wrote:

> This patch lays down the foundation for the kernel memory component
> of the Memory Controller.
> 
> As of today, I am only laying down the following files:
> 
>  * memory.independent_kmem_limit
>  * memory.kmem.limit_in_bytes (currently ignored)
>  * memory.kmem.usage_in_bytes (always zero)
> 
> Signed-off-by: Glauber Costa <glommer@parallels.com>
> CC: Paul Menage <paul@paulmenage.org>
> CC: Greg Thelen <gthelen@google.com>

I'm sorry that my slow review is delaying you.


> ---
>  Documentation/cgroups/memory.txt |   30 +++++++++-
>  init/Kconfig                     |   11 ++++
>  mm/memcontrol.c                  |  115 ++++++++++++++++++++++++++++++++++++--
>  3 files changed, 148 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> index 6f3c598..6f1954a 100644
> --- a/Documentation/cgroups/memory.txt
> +++ b/Documentation/cgroups/memory.txt
> @@ -44,8 +44,9 @@ Features:
>   - oom-killer disable knob and oom-notifier
>   - Root cgroup has no limit controls.
>  
> - Kernel memory and Hugepages are not under control yet. We just manage
> - pages on LRU. To add more controls, we have to take care of performance.
> + Hugepages is not under control yet. We just manage pages on LRU. To add more
> + controls, we have to take care of performance. Kernel memory support is work
> + in progress, and the current version provides basically functionality.
>  
>  Brief summary of control files.
>  
> @@ -56,8 +57,11 @@ Brief summary of control files.
>  				 (See 5.5 for details)
>   memory.memsw.usage_in_bytes	 # show current res_counter usage for memory+Swap
>  				 (See 5.5 for details)
> + memory.kmem.usage_in_bytes	 # show current res_counter usage for kmem only.
> +				 (See 2.7 for details)
>   memory.limit_in_bytes		 # set/show limit of memory usage
>   memory.memsw.limit_in_bytes	 # set/show limit of memory+Swap usage
> + memory.kmem.limit_in_bytes	 # if allowed, set/show limit of kernel memory
>   memory.failcnt			 # show the number of memory usage hits limits
>   memory.memsw.failcnt		 # show the number of memory+Swap hits limits
>   memory.max_usage_in_bytes	 # show max memory usage recorded
> @@ -72,6 +76,9 @@ Brief summary of control files.
>   memory.oom_control		 # set/show oom controls.
>   memory.numa_stat		 # show the number of memory usage per numa node
>  
> + memory.independent_kmem_limit	 # select whether or not kernel memory limits are
> +				   independent of user limits
> +
>  1. History
>  
>  The memory controller has a long history. A request for comments for the memory
> @@ -255,6 +262,25 @@ When oom event notifier is registered, event will be delivered.
>    per-zone-per-cgroup LRU (cgroup's private LRU) is just guarded by
>    zone->lru_lock, it has no lock of its own.
>  
> +2.7 Kernel Memory Extension (CONFIG_CGROUP_MEM_RES_CTLR_KMEM)
> +
> + With the Kernel memory extension, the Memory Controller is able to limit
> +the amount of kernel memory used by the system. Kernel memory is fundamentally
> +different than user memory, since it can't be swapped out, which makes it
> +possible to DoS the system by consuming too much of this precious resource.
> +Kernel memory limits are not imposed for the root cgroup.
> +
> +Memory limits as specified by the standard Memory Controller may or may not
> +take kernel memory into consideration. This is achieved through the file
> +memory.independent_kmem_limit. A Value different than 0 will allow for kernel
> +memory to be controlled separately.
> +
> +When kernel memory limits are not independent, the limit values set in
> +memory.kmem files are ignored.
> +
> +Currently no soft limit is implemented for kernel memory. It is future work
> +to trigger slab reclaim when those limits are reached.
> +
>  3. User Interface
>  
>  0. Configuration
> diff --git a/init/Kconfig b/init/Kconfig
> index d627783..49e5839 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -689,6 +689,17 @@ config CGROUP_MEM_RES_CTLR_SWAP_ENABLED
>  	  For those who want to have the feature enabled by default should
>  	  select this option (if, for some reason, they need to disable it
>  	  then swapaccount=0 does the trick).
> +config CGROUP_MEM_RES_CTLR_KMEM
> +	bool "Memory Resource Controller Kernel Memory accounting"
> +	depends on CGROUP_MEM_RES_CTLR
> +	default y
> +	help
> +	  The Kernel Memory extension for Memory Resource Controller can limit
> +	  the amount of memory used by kernel objects in the system. Those are
> +	  fundamentally different from the entities handled by the standard
> +	  Memory Controller, which are page-based, and can be swapped. Users of
> +	  the kmem extension can use it to guarantee that no group of processes
> +	  will ever exhaust kernel resources alone.
>  
>  config CGROUP_PERF
>  	bool "Enable perf_event per-cpu per-container group (cgroup) monitoring"
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index ebd1e86..d32e931 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -73,7 +73,11 @@ static int really_do_swap_account __initdata = 0;
>  #define do_swap_account		(0)
>  #endif
>  
> -
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
> +int do_kmem_account __read_mostly = 1;
> +#else
> +#define do_kmem_account		0
> +#endif


Hmm, do we really need this boot option ?
>From my experience to have swap-accounting boot option,
this scares us ;) I think config is enough.




>  /*
>   * Statistics for memory cgroup.
>   */
> @@ -270,6 +274,10 @@ struct mem_cgroup {
>  	 */
>  	struct res_counter memsw;
>  	/*
> +	 * the counter to account for kmem usage.
> +	 */
> +	struct res_counter kmem;
> +	/*
>  	 * Per cgroup active and inactive list, similar to the
>  	 * per zone LRU lists.
>  	 */
> @@ -321,6 +329,11 @@ struct mem_cgroup {
>  	 */
>  	unsigned long 	move_charge_at_immigrate;
>  	/*
> +	 * Should kernel memory limits be stabilished independently
> +	 * from user memory ?
> +	 */
> +	int		kmem_independent;
> +	/*
>  	 * percpu counter.
>  	 */
>  	struct mem_cgroup_stat_cpu *stat;
> @@ -388,9 +401,14 @@ enum charge_type {
>  };
>  
>  /* for encoding cft->private value on file */
> -#define _MEM			(0)
> -#define _MEMSWAP		(1)
> -#define _OOM_TYPE		(2)
> +
> +enum mem_type {
> +	_MEM = 0,
> +	_MEMSWAP,
> +	_OOM_TYPE,
> +	_KMEM,
> +};
> +

ok, nice clean up.


>  #define MEMFILE_PRIVATE(x, val)	(((x) << 16) | (val))
>  #define MEMFILE_TYPE(val)	(((val) >> 16) & 0xffff)
>  #define MEMFILE_ATTR(val)	((val) & 0xffff)
> @@ -3943,10 +3961,15 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *mem, bool swap)
>  	u64 val;
>  
>  	if (!mem_cgroup_is_root(mem)) {
> +		val = 0;
> +		if (!mem->kmem_independent)
> +			val = res_counter_read_u64(&mem->kmem, RES_USAGE);

>  		if (!swap)
> -			return res_counter_read_u64(&mem->res, RES_USAGE);
> +			val += res_counter_read_u64(&mem->res, RES_USAGE);
>  		else
> -			return res_counter_read_u64(&mem->memsw, RES_USAGE);
> +			val += res_counter_read_u64(&mem->memsw, RES_USAGE);
> +
> +		return val;
>  	}
>  
>  	val = mem_cgroup_recursive_stat(mem, MEM_CGROUP_STAT_CACHE);
> @@ -3979,6 +4002,10 @@ static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
>  		else
>  			val = res_counter_read_u64(&mem->memsw, name);
>  		break;
> +	case _KMEM:
> +		val = res_counter_read_u64(&mem->kmem, name);
> +		break;
> +
>  	default:
>  		BUG();
>  		break;
> @@ -4756,6 +4783,21 @@ static int mem_cgroup_reset_vmscan_stat(struct cgroup *cgrp,
>  	return 0;
>  }
>  
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
> +static u64 kmem_limit_independent_read(struct cgroup *cont, struct cftype *cft)
> +{
> +	return mem_cgroup_from_cont(cont)->kmem_independent;
> +}
> +
> +static int kmem_limit_independent_write(struct cgroup *cont, struct cftype *cft,
> +					u64 val)
> +{
> +	cgroup_lock();
> +	mem_cgroup_from_cont(cont)->kmem_independent = !!val;
> +	cgroup_unlock();

Hm. This code allows that parent/child can have different settings.
Could you add parent-child check as..

"If parent sets use_hierarchy==1, children must have the same kmem_independent value
with parant's one."

How do you think ? I think a hierarchy must have the same config.


BTW...I don't like naming a little ;)

memory->consolidated/shared/?????_kmem_accounting ?
Or
memory->kmem_independent_accounting ?

or some better naming ?

Thanks,
-Kame

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 2/7] socket: initial cgroup code.
From: KAMEZAWA Hiroyuki @ 2011-09-26 10:52 UTC (permalink / raw)
  To: Glauber Costa
  Cc: Balbir Singh, Greg Thelen, linux-kernel, paul, lizf, ebiederm,
	davem, netdev, linux-mm, kirill
In-Reply-To: <4E7DECF0.9050804@parallels.com>

On Sat, 24 Sep 2011 11:45:04 -0300
Glauber Costa <glommer@parallels.com> wrote:

> On 09/22/2011 12:09 PM, Balbir Singh wrote:
> > On Thu, Sep 22, 2011 at 11:30 AM, Greg Thelen<gthelen@google.com>  wrote:
> >> On Wed, Sep 21, 2011 at 11:59 AM, Glauber Costa<glommer@parallels.com>  wrote:
> >>> Right now I am working under the assumption that tasks are long lived inside
> >>> the cgroup. Migration potentially introduces some nasty locking problems in
> >>> the mem_schedule path.
> >>>
> >>> Also, unless I am missing something, the memcg already has the policy of
> >>> not carrying charges around, probably because of this very same complexity.
> >>>
> >>> True that at least it won't EBUSY you... But I think this is at least a way
> >>> to guarantee that the cgroup under our nose won't disappear in the middle of
> >>> our allocations.
> >>
> >> Here's the memcg user page behavior using the same pattern:
> >>
> >> 1. user page P is allocate by task T in memcg M1
> >> 2. T is moved to memcg M2.  The P charge is left behind still charged
> >> to M1 if memory.move_charge_at_immigrate=0; or the charge is moved to
> >> M2 if memory.move_charge_at_immigrate=1.
> >> 3. rmdir M1 will try to reclaim P (if P was left in M1).  If unable to
> >> reclaim, then P is recharged to parent(M1).
> >>
> >
> > We also have some magic in page_referenced() to remove pages
> > referenced from different containers. What we do is try not to
> > penalize a cgroup if another cgroup is referencing this page and the
> > page under consideration is being reclaimed from the cgroup that
> > touched it.
> >
> > Balbir Singh
> Do you guys see it as a showstopper for this series to be merged, or can 
> we just TODO it ?
> 

In my experience, 'I can't rmdir cgroup.' is always an important/difficult
problem. The users cannot know where the accouting is leaking other than
kmem.usage_in_bytes or memory.usage_in_bytes. and can't fix the issue.

please add EXPERIMENTAL to Kconfig until this is fixed.

> I can push a proposal for it, but it would be done in a separate patch 
> anyway. Also, we may be in better conditions to fix this when the slab 
> part is merged - since it will likely have the same problems...
> 

Yes. considering sockets which can be shared between tasks(cgroups)
you'll finally need
  - owner task of socket 
  - account moving callback

Or disallow task moving once accounted.


Thanks,
-Kame








--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 4/7] per-cgroup tcp buffers control
From: KAMEZAWA Hiroyuki @ 2011-09-26 10:59 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill
In-Reply-To: <1316393805-3005-5-git-send-email-glommer@parallels.com>

On Sun, 18 Sep 2011 21:56:42 -0300
Glauber Costa <glommer@parallels.com> wrote:

> With all the infrastructure in place, this patch implements
> per-cgroup control for tcp memory pressure handling.
> 
> Signed-off-by: Glauber Costa <glommer@parallels.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com>
> CC: Eric W. Biederman <ebiederm@xmission.com>

a comment below.

> +int tcp_init_cgroup(struct proto *prot, struct cgroup *cgrp,
> +		    struct cgroup_subsys *ss)
> +{
> +	struct mem_cgroup *cg = mem_cgroup_from_cont(cgrp);
> +	unsigned long limit;
> +
> +	cg->tcp_memory_pressure = 0;
> +	atomic_long_set(&cg->tcp_memory_allocated, 0);
> +	percpu_counter_init(&cg->tcp_sockets_allocated, 0);
> +
> +	limit = nr_free_buffer_pages() / 8;
> +	limit = max(limit, 128UL);
> +
> +	cg->tcp_prot_mem[0] = sysctl_tcp_mem[0];
> +	cg->tcp_prot_mem[1] = sysctl_tcp_mem[1];
> +	cg->tcp_prot_mem[2] = sysctl_tcp_mem[2];
> +

Then, the parameter doesn't inherit parent's one ?

I think sockets_populate should pass 'parent' and


I think you should have a function 

    mem_cgroup_should_inherit_parent_settings(parent)

(This is because you made this feature as a part of memcg.
 please provide expected behavior.)

Thanks,
-Kame


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 6/7] tcp buffer limitation: per-cgroup limit
From: KAMEZAWA Hiroyuki @ 2011-09-26 11:02 UTC (permalink / raw)
  To: Glauber Costa
  Cc: Greg Thelen, linux-kernel, paul, lizf, ebiederm, davem, netdev,
	linux-mm, kirill
In-Reply-To: <4E7DDB82.3030802@parallels.com>

On Sat, 24 Sep 2011 10:30:42 -0300
Glauber Costa <glommer@parallels.com> wrote:

> On 09/22/2011 03:01 AM, Greg Thelen wrote:
> > On Sun, Sep 18, 2011 at 5:56 PM, Glauber Costa<glommer@parallels.com>  wrote:
> >> +static inline bool mem_cgroup_is_root(struct mem_cgroup *mem)
> >> +{
> >> +       return (mem == root_mem_cgroup);
> >> +}
> >> +
> >
> > Why are you adding a copy of mem_cgroup_is_root().  I see one already
> > in v3.0.  Was it deleted in a previous patch?
> 
> Already answered by another good samaritan.
> 
> >> +static int tcp_write_maxmem(struct cgroup *cgrp, struct cftype *cft, u64 val)
> >> +{
> >> +       struct mem_cgroup *sg = mem_cgroup_from_cont(cgrp);
> >> +       struct mem_cgroup *parent = parent_mem_cgroup(sg);
> >> +       struct net *net = current->nsproxy->net_ns;
> >> +       int i;
> >> +
> >> +       if (!cgroup_lock_live_group(cgrp))
> >> +               return -ENODEV;
> >
> > Why is cgroup_lock_live_cgroup() needed here?  Does it protect updates
> > to sg->tcp_prot_mem[*]?
> >
> >> +static u64 tcp_read_maxmem(struct cgroup *cgrp, struct cftype *cft)
> >> +{
> >> +       struct mem_cgroup *sg = mem_cgroup_from_cont(cgrp);
> >> +       u64 ret;
> >> +
> >> +       if (!cgroup_lock_live_group(cgrp))
> >> +               return -ENODEV;
> >
> > Why is cgroup_lock_live_cgroup() needed here?  Does it protect updates
> > to sg->tcp_max_memory?
> 
> No, that is not my understanding. My understanding is this lock is 
> needed to protect against the cgroup just disappearing under our nose.
> 

Hm. reference count of dentry for cgroup isn't enough ?

Thanks,
-Kame

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox