Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net v2] tg3: Avoid NULL pointer dereference in tg3_io_error_detected()
From: Michael Chan @ 2016-09-29 17:41 UTC (permalink / raw)
  To: Guilherme G. Piccoli
  Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan, Netdev,
	Milton Miller
In-Reply-To: <1475166248-8665-1-git-send-email-gpiccoli@linux.vnet.ibm.com>

On Thu, Sep 29, 2016 at 9:24 AM, Guilherme G. Piccoli
<gpiccoli@linux.vnet.ibm.com> wrote:
> From: Milton Miller <miltonm@us.ibm.com>
>
> While the driver is probing the adapter, an error may occur before the
> netdev structure is allocated and attached to pci_dev. In this case,
> not only netdev isn't available, but the tg3 private structure is also
> not available as it is just math from the NULL pointer, so dereferences
> must be skipped.
>
> The following trace is seen when the error is triggered:
>
>   [1.402247] Unable to handle kernel paging request for data at address 0x00001a99
>   [1.402410] Faulting instruction address: 0xc0000000007e33f8
>   [1.402450] Oops: Kernel access of bad area, sig: 11 [#1]
>   [1.402481] SMP NR_CPUS=2048 NUMA PowerNV
>   [1.402513] Modules linked in:
>   [1.402545] CPU: 0 PID: 651 Comm: eehd Not tainted 4.4.0-36-generic #55-Ubuntu
>   [1.402591] task: c000001fe4e42a20 ti: c000001fe4e88000 task.ti: c000001fe4e88000
>   [1.402742] NIP: c0000000007e33f8 LR: c0000000007e3164 CTR: c000000000595ea0
>   [1.402787] REGS: c000001fe4e8b790 TRAP: 0300   Not tainted  (4.4.0-36-generic)
>   [1.402832] MSR: 9000000100009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 28000422  XER: 20000000
>   [1.403058] CFAR: c000000000008468 DAR: 0000000000001a99 DSISR: 42000000 SOFTE: 1
>   GPR00: c0000000007e3164 c000001fe4e8ba10 c0000000015c5e00 0000000000000000
>   GPR04: 0000000000000001 0000000000000000 0000000000000039 0000000000000299
>   GPR08: 0000000000000000 0000000000000001 c000001fe4e88000 0000000000000006
>   GPR12: 0000000000000000 c00000000fb40000 c0000000000e6558 c000003ca1bffd00
>   GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000000d52768
>   GPR24: c000000000d52740 0000000000000100 c000003ca1b52000 0000000000000002
>   GPR28: 0000000000000900 0000000000000000 c00000000152a0c0 c000003ca1b52000
>   [1.404226] NIP [c0000000007e33f8] tg3_io_error_detected+0x308/0x340
>   [1.404265] LR [c0000000007e3164] tg3_io_error_detected+0x74/0x340
>
> This patch avoids the NULL pointer dereference by moving the access after
> the netdev NULL pointer check on tg3_io_error_detected(). Also, we add a
> check for netdev being NULL on tg3_io_resume() [suggested by Michael Chan].
>
> Fixes: 0486a063b1ff ("tg3: prevent ifup/ifdown during PCI error recovery")
> Fixes: dfc8f370316b ("net/tg3: Release IRQs on permanent error")
> Tested-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>

Acked-by: Michael Chan <michael.chan@broadcom.com>

^ permalink raw reply

* [net-next PATCH] i40e: Clean up handling of msg_enable flags and debug parameter
From: Alexander Duyck @ 2016-09-29 12:05 UTC (permalink / raw)
  To: sassmann, intel-wired-lan, jeffrey.t.kirsher, netdev; +Cc: davem, linville

So the i40e driver had a really convoluted configuration for how to handle
the debug flags contained in msg_enable.  Part of the issue is that the
driver has its own 32 bit mask that it was using to track a separate set of
debug features.  From what I can tell it was trying to use the upper 4 bits
to determine if the value was meant to represent a bit-mask or the numeric
value provided by debug level.

What this patch does is clean this up by compressing those 4 bits into bit
31, as a result we just have to perform a check against the value being
negative to determine if we are looking at a debug level (positive), or a
debug mask (negative).  The debug level will populate the msg_level, and
the debug mask will populate the debug_mask in the hardware struct.

I added similar logic for ethtool.  If the value being provided has bit 31
set we assume the value being provided is a debug mask, otherwise we assume
it is a msg_enable mask.  For displaying we only provide the msg_enable,
and if debug_mask is in use we will print it to the dmesg log.

Lastly I removed the debugfs interface.  It is redundant with what we
already have in ethtool and really doesn't belong anyway.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

So I am running this through a slightly different process than standard
because there are some items here that might be objectionable so I want to
have that ironed out before I deal with the out-of-tree Intel driver.

I just want to verify if this fix for this will work for net-next or if I
need to look at taking a different approach.  Once I get the go-no-go, I
will back-port this into the out-of-tree i40e driver.

 drivers/net/ethernet/intel/i40e/i40e_debugfs.c |   18 ---------------
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c |    7 +++++-
 drivers/net/ethernet/intel/i40e/i40e_main.c    |   28 ++++++++++++------------
 3 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 0c1875b..acb0f13 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -1210,24 +1210,6 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 			dev_info(&pf->pdev->dev,
 				 "dump debug fwdata <cluster_id> <table_id> <index>\n");
 		}
-
-	} else if (strncmp(cmd_buf, "msg_enable", 10) == 0) {
-		u32 level;
-		cnt = sscanf(&cmd_buf[10], "%i", &level);
-		if (cnt) {
-			if (I40E_DEBUG_USER & level) {
-				pf->hw.debug_mask = level;
-				dev_info(&pf->pdev->dev,
-					 "set hw.debug_mask = 0x%08x\n",
-					 pf->hw.debug_mask);
-			}
-			pf->msg_enable = level;
-			dev_info(&pf->pdev->dev, "set msg_enable = 0x%08x\n",
-				 pf->msg_enable);
-		} else {
-			dev_info(&pf->pdev->dev, "msg_enable = 0x%08x\n",
-				 pf->msg_enable);
-		}
 	} else if (strncmp(cmd_buf, "pfr", 3) == 0) {
 		dev_info(&pf->pdev->dev, "debugfs: forcing PFR\n");
 		i40e_do_reset_safe(pf, BIT(__I40E_PF_RESET_REQUESTED));
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index e79a920..b133a77 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -978,6 +978,10 @@ static u32 i40e_get_msglevel(struct net_device *netdev)
 {
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_pf *pf = np->vsi->back;
+	u32 debug_mask = pf->hw.debug_mask;
+
+	if (debug_mask)
+		netdev_info(netdev, "i40e debug_mask: 0x%08X\n", debug_mask);
 
 	return pf->msg_enable;
 }
@@ -989,7 +993,8 @@ static void i40e_set_msglevel(struct net_device *netdev, u32 data)
 
 	if (I40E_DEBUG_USER & data)
 		pf->hw.debug_mask = data;
-	pf->msg_enable = data;
+	else
+		pf->msg_enable = data;
 }
 
 static int i40e_get_regs_len(struct net_device *netdev)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index b93fa2a..b24c6eb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -93,8 +93,8 @@ MODULE_DEVICE_TABLE(pci, i40e_pci_tbl);
 
 #define I40E_MAX_VF_COUNT 128
 static int debug = -1;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+module_param(debug, uint, 0);
+MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all), Debug mask (0x8XXXXXXX)");
 
 MODULE_AUTHOR("Intel Corporation, <e1000-devel@lists.sourceforge.net>");
 MODULE_DESCRIPTION("Intel(R) Ethernet Connection XL710 Network Driver");
@@ -8481,14 +8481,12 @@ static int i40e_sw_init(struct i40e_pf *pf)
 	int err = 0;
 	int size;
 
-	pf->msg_enable = netif_msg_init(I40E_DEFAULT_MSG_ENABLE,
-				(NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK));
-	if (debug != -1 && debug != I40E_DEFAULT_MSG_ENABLE) {
-		if (I40E_DEBUG_USER & debug)
-			pf->hw.debug_mask = debug;
-		pf->msg_enable = netif_msg_init((debug & ~I40E_DEBUG_USER),
-						I40E_DEFAULT_MSG_ENABLE);
-	}
+	pf->msg_enable = netif_msg_init(debug,
+					NETIF_MSG_DRV |
+					NETIF_MSG_PROBE |
+					NETIF_MSG_LINK);
+	if (debug < -1)
+		pf->hw.debug_mask = debug;
 
 	/* Set default capability flags */
 	pf->flags = I40E_FLAG_RX_CSUM_ENABLED |
@@ -10790,10 +10788,12 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	mutex_init(&hw->aq.asq_mutex);
 	mutex_init(&hw->aq.arq_mutex);
 
-	if (debug != -1) {
-		pf->msg_enable = pf->hw.debug_mask;
-		pf->msg_enable = debug;
-	}
+	pf->msg_enable = netif_msg_init(debug,
+					NETIF_MSG_DRV |
+					NETIF_MSG_PROBE |
+					NETIF_MSG_LINK);
+	if (debug < -1)
+		pf->hw.debug_mask = debug;
 
 	/* do a special CORER for clearing PXE mode once at init */
 	if (hw->revision_id == 0 &&

^ permalink raw reply related

* Re: pull-request: wireless-drivers-next 2016-09-29
From: Pablo Neira Ayuso @ 2016-09-29 18:21 UTC (permalink / raw)
  To: Kalle Valo
  Cc: David Miller, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87ponmve2v.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>

On Thu, Sep 29, 2016 at 07:57:28PM +0300, Kalle Valo wrote:
> Hi Dave,
> 
> this should be the last wireless-drivers-next pull request for 4.9, from
> now on only important bugfixes. Nothing really special stands out,
> iwlwifi being most active but other drivers also getting attention. More
> details in the signed tag. Please let me know if there are any problems.
> 
> Or actually I had one problem. While doing a test merge I noticed that
> net-next fails to compile for me, but I don't think this is anything
> wireless related:
> 
>   CC      net/netfilter/core.o
> net/netfilter/core.c: In function 'nf_set_hooks_head':
> net/netfilter/core.c:96:149: error: 'struct net_device' has no member named 'nf_hooks_ingress'

That's my problem, will be sending a pull request to fix this asap,
thanks.

^ permalink raw reply

* Re: next-20160929 build: 2 failures 4 warnings (next-20160929)
From: Mark Brown @ 2016-09-29 18:47 UTC (permalink / raw)
  To: Vishwanath Pai, Joshua Hunt, Pablo Neira Ayuso
  Cc: kernel-build-reports, linaro-kernel, linux-next, netdev,
	linux-arm-kernel
In-Reply-To: <E1bpZhb-0002Oz-OQ@optimist>

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

On Thu, Sep 29, 2016 at 12:40:35PM +0100, Build bot for Mark Brown wrote:

For the past couple of days -next has been failing to build an ARM
allmodconfig due to:

> 	arm-allmodconfig
> ERROR: "__aeabi_uldivmod" [net/netfilter/xt_hashlimit.ko] undefined!

which appears to be triggered by 11d5f15723c9 (netfilter: xt_hashlimit:
Create revision 2 to support higher pps rates) introducing a division of
a 64 bit number which should be done using do_div().

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: next-20160929 build: 2 failures 4 warnings (next-20160929)
From: Vishwanath Pai @ 2016-09-29 19:08 UTC (permalink / raw)
  To: Mark Brown
  Cc: Joshua Hunt, Pablo Neira Ayuso, kernel-build-reports,
	linaro-kernel, linux-next, netdev, linux-arm-kernel
In-Reply-To: <20160929184737.ragquwy2tmhxo5bn@sirena.org.uk>

On 09/29/2016 02:47 PM, Mark Brown wrote:
> On Thu, Sep 29, 2016 at 12:40:35PM +0100, Build bot for Mark Brown wrote:
> 
> For the past couple of days -next has been failing to build an ARM
> allmodconfig due to:
> 
>> 	arm-allmodconfig
>> ERROR: "__aeabi_uldivmod" [net/netfilter/xt_hashlimit.ko] undefined!
> 
> which appears to be triggered by 11d5f15723c9 (netfilter: xt_hashlimit:
> Create revision 2 to support higher pps rates) introducing a division of
> a 64 bit number which should be done using do_div().
> 

I have sent a patch for this a couple of days ago to netdev, it hasn't
made it to net-next yet. Here's the latest one:

[PATCH net-next v3] netfilter: xt_hashlimit: Fix link error in 32bit
arch because of 64bit division

This should fix the link error.

-Vishwanath

^ permalink raw reply

* [PATCH net-next 0/2] openvswitch: mpls fix and clean up
From: Jiri Benc @ 2016-09-29 19:19 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, pravin shelar

Convert to the new mpls skb layout the last remaining place in openvswitch,
forgotten on the mpls GSO rework. The GSO rework also allows for some nice
cleanup in the second patch.

Jiri Benc (2):
  openvswitch: mpls: set network header correctly on key extract
  openvswitch: remove skb_mpls_header

 include/net/mpls.h        | 11 -----------
 net/openvswitch/actions.c | 10 +++++-----
 net/openvswitch/flow.c    | 11 +++--------
 3 files changed, 8 insertions(+), 24 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH net-next 1/2] openvswitch: mpls: set network header correctly on key extract
From: Jiri Benc @ 2016-09-29 19:19 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, pravin shelar
In-Reply-To: <cover.1475176437.git.jbenc@redhat.com>

After the 48d2ab609b6b ("net: mpls: Fixups for GSO"), MPLS handling in
openvswitch was changed to have network header pointing to the start of the
MPLS headers and inner_network_header pointing after the MPLS headers.

However, key_extract was missed by the mentioned commit, causing incorrect
headers to be set when a MPLS packet just enters the bridge or after it is
recirculated.

Fixes: 48d2ab609b6b ("net: mpls: Fixups for GSO")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 net/openvswitch/flow.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 634cc10d6dee..c8c82e109c68 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -633,12 +633,7 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
 	} else if (eth_p_mpls(key->eth.type)) {
 		size_t stack_len = MPLS_HLEN;
 
-		/* In the presence of an MPLS label stack the end of the L2
-		 * header and the beginning of the L3 header differ.
-		 *
-		 * Advance network_header to the beginning of the L3
-		 * header. mac_len corresponds to the end of the L2 header.
-		 */
+		skb_set_inner_network_header(skb, skb->mac_len);
 		while (1) {
 			__be32 lse;
 
@@ -646,12 +641,12 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
 			if (unlikely(error))
 				return 0;
 
-			memcpy(&lse, skb_network_header(skb), MPLS_HLEN);
+			memcpy(&lse, skb_inner_network_header(skb), MPLS_HLEN);
 
 			if (stack_len == MPLS_HLEN)
 				memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
 
-			skb_set_network_header(skb, skb->mac_len + stack_len);
+			skb_set_inner_network_header(skb, skb->mac_len + stack_len);
 			if (lse & htonl(MPLS_LS_S_MASK))
 				break;
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 2/2] openvswitch: remove skb_mpls_header
From: Jiri Benc @ 2016-09-29 19:19 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, pravin shelar
In-Reply-To: <cover.1475176437.git.jbenc@redhat.com>

skb_mpls_header is equivalent to skb_network_header now. There's no reason
to keep it.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 include/net/mpls.h        | 11 -----------
 net/openvswitch/actions.c | 10 +++++-----
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/include/net/mpls.h b/include/net/mpls.h
index 5b3b5addfb08..fde22d0b0ec1 100644
--- a/include/net/mpls.h
+++ b/include/net/mpls.h
@@ -25,15 +25,4 @@ static inline bool eth_p_mpls(__be16 eth_type)
 		eth_type == htons(ETH_P_MPLS_MC);
 }
 
-/*
- * For non-MPLS skbs this will correspond to the network header.
- * For MPLS skbs it will be before the network_header as the MPLS
- * label stack lies between the end of the mac header and the network
- * header. That is, for MPLS skbs the end of the mac header
- * is the top of the MPLS label stack.
- */
-static inline unsigned char *skb_mpls_header(struct sk_buff *skb)
-{
-	return skb_mac_header(skb) + skb->mac_len;
-}
 #endif
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 863e992dfbc0..bf4211f5c974 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -180,7 +180,7 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	skb_reset_mac_header(skb);
 	skb_set_network_header(skb, skb->mac_len);
 
-	new_mpls_lse = (__be32 *)skb_mpls_header(skb);
+	new_mpls_lse = (__be32 *)skb_network_header(skb);
 	*new_mpls_lse = mpls->mpls_lse;
 
 	skb_postpush_rcsum(skb, new_mpls_lse, MPLS_HLEN);
@@ -202,7 +202,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	if (unlikely(err))
 		return err;
 
-	skb_postpull_rcsum(skb, skb_mpls_header(skb), MPLS_HLEN);
+	skb_postpull_rcsum(skb, skb_network_header(skb), MPLS_HLEN);
 
 	memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
 		skb->mac_len);
@@ -211,10 +211,10 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	skb_reset_mac_header(skb);
 	skb_set_network_header(skb, skb->mac_len);
 
-	/* skb_mpls_header() is used to locate the ethertype
+	/* skb_network_header() is used to locate the ethertype
 	 * field correctly in the presence of VLAN tags.
 	 */
-	hdr = (struct ethhdr *)(skb_mpls_header(skb) - ETH_HLEN);
+	hdr = (struct ethhdr *)(skb_network_header(skb) - ETH_HLEN);
 	update_ethertype(skb, hdr, ethertype);
 	if (eth_p_mpls(skb->protocol))
 		skb->protocol = ethertype;
@@ -234,7 +234,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
 	if (unlikely(err))
 		return err;
 
-	stack = (__be32 *)skb_mpls_header(skb);
+	stack = (__be32 *)skb_network_header(skb);
 	lse = OVS_MASKED(*stack, *mpls_lse, *mask);
 	if (skb->ip_summed == CHECKSUM_COMPLETE) {
 		__be32 diff[] = { ~(*stack), lse };
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Tom Herbert @ 2016-09-29 20:26 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, Linux Kernel Network Developers, Kernel Team,
	Rick Jones, Alexander Duyck
In-Reply-To: <1475162142.28155.188.camel@edumazet-glaptop3.roam.corp.google.com>

On Thu, Sep 29, 2016 at 11:15 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2016-09-29 at 07:51 -0700, Eric Dumazet wrote:
>> On Thu, 2016-09-29 at 10:08 -0400, Tom Herbert wrote:
>>
>> > It addresses  the issue that Rick Jones pointed out was happening with
>> > XPS. When packets are sent for a flow that has no socket and XPS is
>> > enabled then each packet uses the XPS queue based on the running CPU.
>> > Since the thread sending on a flow can be rescheduled on different
>> > CPUs this is creating ooo packets. In this case the ooo is being
>> > caused by interaction with XPS.
>> >
>>
>> Nope, your patch does not address the problem properly.
>>
>> I am not sure I want to spend more time explaining the issue.
>>
>> Lets talk about this in Tokyo next week.
>>
>
> Just as a reminder, sorry to bother you, stating some obvious facts for
> both of us. We have public exchanges, so we also need to re-explain how
> things work.
>
> Queue selection on xmit happens before we hit the qdisc and its delays.
>
> So when you access txq->dql.num_completed_ops and
> txq->dql.num_enqueue_ops you can observe values that do not change for a
> while.
>
> Say a thread runs on a VM, and sends 2 packets P1, P2 on the same flow
> (skb_get_hash() returns the same value for these 2 packets)
>
> P1 is sent on behalf of CPU 1, we pickup queue txq1, and queue the
> packet on its qdisc . Transmit does not happen because of some
> constraints like rate limiting or scheduling constraints.
>
> P2 is sent on behalf of CPU 2, we pickup queue txq2, notice that prior
> packet chose txq1. We check txq1->dql and decide it is fine to use txq2,
> since the dql params of txq1 were not changed yet.
>
> ( txq->dql.num_completed_ops == ent.queue_ptr )
>
> Note that in RFS case, we have the guarantee that we observe 'live
> queues' since they are the per cpu backlog.
>
> So input_queue_head_incr() and input_queue_tail_incr_save() are
> correctly doing the OOO prevention, because a queued packet immediately
> changes the state.
>
> So really your patch works if you have no qdisc, or a non congested
> qdisc. (Think if P1 is dropped by a full pfifo or pfifo_fast : We really
> want to avoid steering P2, P3, ..., PN on this full pfifo while maybe
> other txq are idle). Strange attractors are back (check commit
> 9b462d02d6dd6 )
>
Understood.

> You could avoid (ab)using BQL with a different method, grabbing
> skb->destructor for the packets that are socketless
>
> The hash table would simply track the sum of skb->truesize to allow flow
> migration. This would be self contained and not intrusive.
>
Okay, will look that.

>
>
>

^ permalink raw reply

* Re: [PATCH] brcmfmac: implement more accurate skb tracking
From: Rafał Miłecki @ 2016-09-29 21:57 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
	Franky Lin,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
	Network Development, Linux Kernel Mailing List,
	Rafał Miłecki
In-Reply-To: <0a070079-e3af-ab5c-0485-744bec8b3d86-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On 27 September 2016 at 11:24, Arend Van Spriel
<arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On 26-9-2016 14:38, Rafał Miłecki wrote:
>> On 26 September 2016 at 14:13, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On 26 September 2016 at 13:46, Arend Van Spriel
>>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>> On 26-9-2016 12:23, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>
>>>>> We need to track 802.1x packets to know if there are any pending ones
>>>>> for transmission. This is required for performing key update in the
>>>>> firmware.
>>>>
>>>> The problem we are trying to solve is a pretty old one. The problem is
>>>> that wpa_supplicant uses two separate code paths: EAPOL messaging
>>>> through data path and key configuration though nl80211.
>>>
>>> Can I find it described/reported somewhere?
>>>
>>>
>>>>> Unfortunately our old tracking code wasn't very accurate. It was
>>>>> treating skb as pending as soon as it was passed by the netif. Actual
>>>>> handling packet to the firmware was happening later as brcmfmac
>>>>> internally queues them and uses its own worker(s).
>>>>
>>>> That does not seem right. As soon as we get a 1x packet we need to wait
>>>> with key configuration regardless whether it is still in the driver or
>>>> handed over to firmware already.
>>>
>>> OK, thanks.
>>
>> Actually, it's not OK. I was trying to report/describe/discuss this
>> problem for over a week. I couldn't get much of answer from you.
>>
>> I had to come with a patch I worked on for quite some time. Only then
>> you decided to react and reply with a reason for a nack. I see this
>> patch may be wrong (but it's still hard to know what's going wrong
>> without a proper hostapd bug report). I'd expect you to somehow work &
>> communicate with open source community.
>
> We do or at least make an honest attempt, but there is more on our plate
> so responses may be delayed. It also does not help when you get anal and
> preachy when we do respond. Also not OK. In this case the delay is
> caused because I had to pick up the thread(s) as Hante is on vacation
> (he needed a break :-p ). However, you started sending patches so I
> decided to look at and respond to those. Sorry if you felt like we left
> you hanging to dry.

I believe I get easily irritated due to my communication experience I
got so far :(


Over a year ago I reported brcmfmac can't recover from failed
register_netdev(ice). This bug remains unfixed.

In 2014 I reported problem with 80 MHz support. I didn't have hardware
to fix & test it on my own (you weren't able/allowed to send me one of
your PCIe cards). In remained broken until I fixed it year later.

You missed my crash bug report about caused by missing eth_type_trans
and came with patch on your own a month later.

Earlier this year I reported you problem with BCM4366 and multiple
interfaces. I didn't get much help. 3 months later I came with patch
to workaround the problem but you said there's a better way to do
this. It took me 2 weeks to figure out a new wlioctl API for that
while all I needed was a simple hint on "interface_remove".

Right now I'm waiting to get any answer from you about 4366c0
firmware. It's still less than 2 weeks since I asked for it, but a
simple ETA would be nice. I'm actually not sure if I should report
more problems to you to don't distract you from pending things.

Problems with brcmf_netdev_wait_pend8021x were reported multiples
times for last few months. When I finally got time for that it took me
a week to debug them.


As you can see, it takes me months to get help on some things. And in
few cases I never got much help at all. Yes, I was hoping to have you
more involved into brcmfmac development and problems solving. I guess
things didn't meet my expectations and I got grumpy & preachy.

-- 
Rafał

^ permalink raw reply

* [PATCH net-next 0/6] rxrpc: Fixes and adjustments
From: David Howells @ 2016-09-29 22:15 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel


This set of patches contains some fixes and adjustments:

 (1) Connections for exclusive calls are being reused because the check to
     work out whether to set RXRPC_CONN_DONT_REUSE is checking where the
     parameters will be copied to (but haven't yet).

 (2) Make Tx loss-injection go through the normal return, so the state gets
     set correctly for what the code thinks it has done.

     Note lost Tx packets in the tx_data trace rather than the skb
     tracepoint.

 (3) Activate channels according to the current state from within the
     channel_lock to avoid someone changing it on us.

 (4) Reduce the local endpoint's services list to a single pointer as we
     don't allow service AF_RXRPC sockets to share UDP ports with other
     AF_RXRPC sockets, so there can't be more than one element in the list.

 (5) Request more ACKs in slow-start mode to help monitor the state driving
     the window configuration.

 (6) Display the serial number of the packet being ACK'd rather than the
     ACK packet's own serial number in the congestion trace as this can be
     related to other entries in the trace.

The patches can be found here also:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-rewrite

Tagged thusly:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	rxrpc-rewrite-20160929

David
---
David Howells (6):
      rxrpc: Fix exclusive client connections
      rxrpc: Make Tx loss-injection go through normal return and adjust tracing
      rxrpc: When activating client conn channels, do state check inside lock
      rxrpc: Reduce the rxrpc_local::services list to a pointer
      rxrpc: Request more ACKs in slow-start mode
      rxrpc: Note serial number being ACK'd in the congestion management trace


 include/trace/events/rxrpc.h |    6 ++++--
 net/rxrpc/af_rxrpc.c         |   21 ++++++++-------------
 net/rxrpc/ar-internal.h      |    6 ++----
 net/rxrpc/call_accept.c      |    8 ++++----
 net/rxrpc/call_event.c       |    2 +-
 net/rxrpc/conn_client.c      |   38 ++++++++++++++++++++++++++------------
 net/rxrpc/input.c            |    8 ++++----
 net/rxrpc/local_object.c     |    3 +--
 net/rxrpc/misc.c             |    1 -
 net/rxrpc/output.c           |   18 ++++++++++--------
 net/rxrpc/security.c         |    8 ++++----
 net/rxrpc/sendmsg.c          |    2 +-
 net/rxrpc/skbuff.c           |   11 +++--------
 13 files changed, 68 insertions(+), 64 deletions(-)

^ permalink raw reply

* [PATCH net-next 1/6] rxrpc: Fix exclusive client connections
From: David Howells @ 2016-09-29 22:15 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147518732742.29235.11597637573731081178.stgit@warthog.procyon.org.uk>

Exclusive connections are currently reusable (which they shouldn't be)
because rxrpc_alloc_client_connection() checks the exclusive flag in the
rxrpc_connection struct before it's initialised from the function
parameters.  This means that the DONT_REUSE flag doesn't get set.

Fix this by checking the function parameters for the exclusive flag.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/conn_client.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index c76a125df891..f5ee8bfa5bef 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -200,7 +200,7 @@ rxrpc_alloc_client_connection(struct rxrpc_conn_parameters *cp, gfp_t gfp)
 	}
 
 	atomic_set(&conn->usage, 1);
-	if (conn->params.exclusive)
+	if (cp->exclusive)
 		__set_bit(RXRPC_CONN_DONT_REUSE, &conn->flags);
 
 	conn->params		= *cp;

^ permalink raw reply related

* [PATCH net-next 2/6] rxrpc: Make Tx loss-injection go through normal return and adjust tracing
From: David Howells @ 2016-09-29 22:15 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147518732742.29235.11597637573731081178.stgit@warthog.procyon.org.uk>

In rxrpc_send_data_packet() make the loss-injection path return through the
same code as the transmission path so that the RTT determination is
initiated and any future timer shuffling will be done, despite the packet
having been binned.

Whilst we're at it:

 (1) Add to the tx_data tracepoint an indication of whether or not we're
     retransmitting a data packet.

 (2) When we're deciding whether or not to request an ACK, rather than
     checking if we're in fast-retransmit mode check instead if we're
     retransmitting.

 (3) Don't invoke the lose_skb tracepoint when losing a Tx packet as we're
     not altering the sk_buff refcount nor are we just seeing it after
     getting it off the Tx list.

 (4) The rxrpc_skb_tx_lost note is then no longer used so remove it.

 (5) rxrpc_lose_skb() no longer needs to deal with rxrpc_skb_tx_lost.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/trace/events/rxrpc.h |    6 ++++--
 net/rxrpc/ar-internal.h      |    3 +--
 net/rxrpc/call_event.c       |    2 +-
 net/rxrpc/misc.c             |    1 -
 net/rxrpc/output.c           |   17 +++++++++--------
 net/rxrpc/sendmsg.c          |    2 +-
 net/rxrpc/skbuff.c           |   11 +++--------
 7 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index ada12d00118c..8ba8d76e856a 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -258,15 +258,16 @@ TRACE_EVENT(rxrpc_rx_ack,
 
 TRACE_EVENT(rxrpc_tx_data,
 	    TP_PROTO(struct rxrpc_call *call, rxrpc_seq_t seq,
-		     rxrpc_serial_t serial, u8 flags, bool lose),
+		     rxrpc_serial_t serial, u8 flags, bool retrans, bool lose),
 
-	    TP_ARGS(call, seq, serial, flags, lose),
+	    TP_ARGS(call, seq, serial, flags, retrans, lose),
 
 	    TP_STRUCT__entry(
 		    __field(struct rxrpc_call *,	call		)
 		    __field(rxrpc_seq_t,		seq		)
 		    __field(rxrpc_serial_t,		serial		)
 		    __field(u8,				flags		)
+		    __field(bool,			retrans		)
 		    __field(bool,			lose		)
 			     ),
 
@@ -275,6 +276,7 @@ TRACE_EVENT(rxrpc_tx_data,
 		    __entry->seq = seq;
 		    __entry->serial = serial;
 		    __entry->flags = flags;
+		    __entry->retrans = retrans;
 		    __entry->lose = lose;
 			   ),
 
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index ca96e547cb9a..6aadaa7d8b43 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -603,7 +603,6 @@ enum rxrpc_skb_trace {
 	rxrpc_skb_tx_cleaned,
 	rxrpc_skb_tx_freed,
 	rxrpc_skb_tx_got,
-	rxrpc_skb_tx_lost,
 	rxrpc_skb_tx_new,
 	rxrpc_skb_tx_rotated,
 	rxrpc_skb_tx_seen,
@@ -1073,7 +1072,7 @@ extern const s8 rxrpc_ack_priority[];
  * output.c
  */
 int rxrpc_send_call_packet(struct rxrpc_call *, u8);
-int rxrpc_send_data_packet(struct rxrpc_call *, struct sk_buff *);
+int rxrpc_send_data_packet(struct rxrpc_call *, struct sk_buff *, bool);
 void rxrpc_reject_packets(struct rxrpc_local *);
 
 /*
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index 0e8478012212..1f6c7633b964 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -256,7 +256,7 @@ static void rxrpc_resend(struct rxrpc_call *call)
 		rxrpc_get_skb(skb, rxrpc_skb_tx_got);
 		spin_unlock_bh(&call->lock);
 
-		if (rxrpc_send_data_packet(call, skb) < 0) {
+		if (rxrpc_send_data_packet(call, skb, true) < 0) {
 			rxrpc_free_skb(skb, rxrpc_skb_tx_freed);
 			return;
 		}
diff --git a/net/rxrpc/misc.c b/net/rxrpc/misc.c
index aedb8978226d..47dddacdbb91 100644
--- a/net/rxrpc/misc.c
+++ b/net/rxrpc/misc.c
@@ -108,7 +108,6 @@ const char rxrpc_skb_traces[rxrpc_skb__nr_trace][7] = {
 	[rxrpc_skb_tx_cleaned]		= "Tx CLN",
 	[rxrpc_skb_tx_freed]		= "Tx FRE",
 	[rxrpc_skb_tx_got]		= "Tx GOT",
-	[rxrpc_skb_tx_lost]		= "Tx *L*",
 	[rxrpc_skb_tx_new]		= "Tx NEW",
 	[rxrpc_skb_tx_rotated]		= "Tx ROT",
 	[rxrpc_skb_tx_seen]		= "Tx SEE",
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index cf43a715685e..ac9a58b619a6 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -238,7 +238,8 @@ out:
 /*
  * send a packet through the transport endpoint
  */
-int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb)
+int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
+			   bool retrans)
 {
 	struct rxrpc_connection *conn = call->conn;
 	struct rxrpc_wire_header whdr;
@@ -247,6 +248,7 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb)
 	struct kvec iov[2];
 	rxrpc_serial_t serial;
 	size_t len;
+	bool lost = false;
 	int ret, opt;
 
 	_enter(",{%d}", skb->len);
@@ -281,7 +283,7 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb)
 	/* If our RTT cache needs working on, request an ACK.  Also request
 	 * ACKs if a DATA packet appears to have been lost.
 	 */
-	if (call->cong_mode == RXRPC_CALL_FAST_RETRANSMIT ||
+	if (retrans ||
 	    (call->peer->rtt_usage < 3 && sp->hdr.seq & 1) ||
 	    ktime_before(ktime_add_ms(call->peer->rtt_last_req, 1000),
 			 ktime_get_real()))
@@ -290,11 +292,9 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb)
 	if (IS_ENABLED(CONFIG_AF_RXRPC_INJECT_LOSS)) {
 		static int lose;
 		if ((lose++ & 7) == 7) {
-			trace_rxrpc_tx_data(call, sp->hdr.seq, serial,
-					    whdr.flags, true);
-			rxrpc_lose_skb(skb, rxrpc_skb_tx_lost);
-			_leave(" = 0 [lose]");
-			return 0;
+			ret = 0;
+			lost = true;
+			goto done;
 		}
 	}
 
@@ -319,7 +319,8 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb)
 		goto send_fragmentable;
 
 done:
-	trace_rxrpc_tx_data(call, sp->hdr.seq, serial, whdr.flags, false);
+	trace_rxrpc_tx_data(call, sp->hdr.seq, serial, whdr.flags,
+			    retrans, lost);
 	if (ret >= 0) {
 		ktime_t now = ktime_get_real();
 		skb->tstamp = now;
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 1f8040d82395..d8dfdce874d8 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -144,7 +144,7 @@ static void rxrpc_queue_packet(struct rxrpc_call *call, struct sk_buff *skb,
 	if (seq == 1 && rxrpc_is_client_call(call))
 		rxrpc_expose_client_call(call);
 
-	ret = rxrpc_send_data_packet(call, skb);
+	ret = rxrpc_send_data_packet(call, skb, false);
 	if (ret < 0) {
 		_debug("need instant resend %d", ret);
 		rxrpc_instant_resend(call, ix);
diff --git a/net/rxrpc/skbuff.c b/net/rxrpc/skbuff.c
index 5154cbf7e540..67b02c45271b 100644
--- a/net/rxrpc/skbuff.c
+++ b/net/rxrpc/skbuff.c
@@ -77,14 +77,9 @@ void rxrpc_lose_skb(struct sk_buff *skb, enum rxrpc_skb_trace op)
 	if (skb) {
 		int n;
 		CHECK_SLAB_OKAY(&skb->users);
-		if (op == rxrpc_skb_tx_lost) {
-			n = atomic_read(select_skb_count(op));
-			trace_rxrpc_skb(skb, op, atomic_read(&skb->users), n, here);
-		} else {
-			n = atomic_dec_return(select_skb_count(op));
-			trace_rxrpc_skb(skb, op, atomic_read(&skb->users), n, here);
-			kfree_skb(skb);
-		}
+		n = atomic_dec_return(select_skb_count(op));
+		trace_rxrpc_skb(skb, op, atomic_read(&skb->users), n, here);
+		kfree_skb(skb);
 	}
 }
 

^ permalink raw reply related

* [PATCH net-next 3/6] rxrpc: When activating client conn channels, do state check inside lock
From: David Howells @ 2016-09-29 22:15 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147518732742.29235.11597637573731081178.stgit@warthog.procyon.org.uk>

In rxrpc_activate_channels(), the connection cache state is checked outside
of the lock, which means it can change whilst we're waking calls up,
thereby changing whether or not we're allowed to wake calls up.

Fix this by moving the check inside the locked region.  The check to see if
all the channels are currently busy can stay outside of the locked region.

Whilst we're at it:

 (1) Split the locked section out into its own function so that we can call
     it from other places in a later patch.

 (2) Determine the mask of channels dependent on the state as we're going
     to add another state in a later patch that will restrict the number of
     simultaneous calls to 1 on a connection.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/conn_client.c |   36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index f5ee8bfa5bef..60ef9605167e 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -576,28 +576,42 @@ static void rxrpc_activate_one_channel(struct rxrpc_connection *conn,
 }
 
 /*
+ * Assign channels and callNumbers to waiting calls with channel_lock
+ * held by caller.
+ */
+static void rxrpc_activate_channels_locked(struct rxrpc_connection *conn)
+{
+	u8 avail, mask;
+
+	switch (conn->cache_state) {
+	case RXRPC_CONN_CLIENT_ACTIVE:
+		mask = RXRPC_ACTIVE_CHANS_MASK;
+		break;
+	default:
+		return;
+	}
+
+	while (!list_empty(&conn->waiting_calls) &&
+	       (avail = ~conn->active_chans,
+		avail &= mask,
+		avail != 0))
+		rxrpc_activate_one_channel(conn, __ffs(avail));
+}
+
+/*
  * Assign channels and callNumbers to waiting calls.
  */
 static void rxrpc_activate_channels(struct rxrpc_connection *conn)
 {
-	unsigned char mask;
-
 	_enter("%d", conn->debug_id);
 
 	trace_rxrpc_client(conn, -1, rxrpc_client_activate_chans);
 
-	if (conn->cache_state != RXRPC_CONN_CLIENT_ACTIVE ||
-	    conn->active_chans == RXRPC_ACTIVE_CHANS_MASK)
+	if (conn->active_chans == RXRPC_ACTIVE_CHANS_MASK)
 		return;
 
 	spin_lock(&conn->channel_lock);
-
-	while (!list_empty(&conn->waiting_calls) &&
-	       (mask = ~conn->active_chans,
-		mask &= RXRPC_ACTIVE_CHANS_MASK,
-		mask != 0))
-		rxrpc_activate_one_channel(conn, __ffs(mask));
-
+	rxrpc_activate_channels_locked(conn);
 	spin_unlock(&conn->channel_lock);
 	_leave("");
 }

^ permalink raw reply related

* [PATCH net-next 4/6] rxrpc: Reduce the rxrpc_local::services list to a pointer
From: David Howells @ 2016-09-29 22:15 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147518732742.29235.11597637573731081178.stgit@warthog.procyon.org.uk>

Reduce the rxrpc_local::services list to just a pointer as we don't permit
multiple service endpoints to bind to a single transport endpoints (this is
excluded by rxrpc_lookup_local()).

The reason we don't allow this is that if you send a request to an AFS
filesystem service, it will try to talk back to your cache manager on the
port you sent from (this is how file change notifications are handled).  To
prevent someone from stealing your CM callbacks, we don't let AF_RXRPC
sockets share a UDP socket if at least one of them has a service bound.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/af_rxrpc.c     |   21 ++++++++-------------
 net/rxrpc/ar-internal.h  |    3 +--
 net/rxrpc/call_accept.c  |    8 ++++----
 net/rxrpc/local_object.c |    3 +--
 net/rxrpc/security.c     |    8 ++++----
 5 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 8dbf7bed2cc4..44c9c2b0b190 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -136,7 +136,8 @@ static int rxrpc_bind(struct socket *sock, struct sockaddr *saddr, int len)
 	struct sockaddr_rxrpc *srx = (struct sockaddr_rxrpc *)saddr;
 	struct sock *sk = sock->sk;
 	struct rxrpc_local *local;
-	struct rxrpc_sock *rx = rxrpc_sk(sk), *prx;
+	struct rxrpc_sock *rx = rxrpc_sk(sk);
+	u16 service_id = srx->srx_service;
 	int ret;
 
 	_enter("%p,%p,%d", rx, saddr, len);
@@ -160,15 +161,12 @@ static int rxrpc_bind(struct socket *sock, struct sockaddr *saddr, int len)
 		goto error_unlock;
 	}
 
-	if (rx->srx.srx_service) {
+	if (service_id) {
 		write_lock(&local->services_lock);
-		hlist_for_each_entry(prx, &local->services, listen_link) {
-			if (prx->srx.srx_service == rx->srx.srx_service)
-				goto service_in_use;
-		}
-
+		if (rcu_access_pointer(local->service))
+			goto service_in_use;
 		rx->local = local;
-		hlist_add_head_rcu(&rx->listen_link, &local->services);
+		rcu_assign_pointer(local->service, rx);
 		write_unlock(&local->services_lock);
 
 		rx->sk.sk_state = RXRPC_SERVER_BOUND;
@@ -599,7 +597,6 @@ static int rxrpc_create(struct net *net, struct socket *sock, int protocol,
 	rx->family = protocol;
 	rx->calls = RB_ROOT;
 
-	INIT_HLIST_NODE(&rx->listen_link);
 	spin_lock_init(&rx->incoming_lock);
 	INIT_LIST_HEAD(&rx->sock_calls);
 	INIT_LIST_HEAD(&rx->to_be_accepted);
@@ -681,11 +678,9 @@ static int rxrpc_release_sock(struct sock *sk)
 	sk->sk_state = RXRPC_CLOSE;
 	spin_unlock_bh(&sk->sk_receive_queue.lock);
 
-	ASSERTCMP(rx->listen_link.next, !=, LIST_POISON1);
-
-	if (!hlist_unhashed(&rx->listen_link)) {
+	if (rx->local && rx->local->service == rx) {
 		write_lock(&rx->local->services_lock);
-		hlist_del_rcu(&rx->listen_link);
+		rx->local->service = NULL;
 		write_unlock(&rx->local->services_lock);
 	}
 
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 6aadaa7d8b43..539db54697f9 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -93,7 +93,6 @@ struct rxrpc_sock {
 	rxrpc_notify_new_call_t	notify_new_call; /* Func to notify of new call */
 	rxrpc_discard_new_call_t discard_new_call; /* Func to discard a new call */
 	struct rxrpc_local	*local;		/* local endpoint */
-	struct hlist_node	listen_link;	/* link in the local endpoint's listen list */
 	struct rxrpc_backlog	*backlog;	/* Preallocation for services */
 	spinlock_t		incoming_lock;	/* Incoming call vs service shutdown lock */
 	struct list_head	sock_calls;	/* List of calls owned by this socket */
@@ -216,7 +215,7 @@ struct rxrpc_local {
 	struct list_head	link;
 	struct socket		*socket;	/* my UDP socket */
 	struct work_struct	processor;
-	struct hlist_head	services;	/* services listening on this endpoint */
+	struct rxrpc_sock __rcu	*service;	/* Service(s) listening on this endpoint */
 	struct rw_semaphore	defrag_sem;	/* control re-enablement of IP DF bit */
 	struct sk_buff_head	reject_queue;	/* packets awaiting rejection */
 	struct sk_buff_head	event_queue;	/* endpoint event packets awaiting processing */
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index a8d39d7cf42c..3cac231d8405 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -331,14 +331,14 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 	struct rxrpc_sock *rx;
 	struct rxrpc_call *call;
+	u16 service_id = sp->hdr.serviceId;
 
 	_enter("");
 
 	/* Get the socket providing the service */
-	hlist_for_each_entry_rcu_bh(rx, &local->services, listen_link) {
-		if (rx->srx.srx_service == sp->hdr.serviceId)
-			goto found_service;
-	}
+	rx = rcu_dereference(local->service);
+	if (service_id == rx->srx.srx_service)
+		goto found_service;
 
 	trace_rxrpc_abort("INV", sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
 			  RX_INVALID_OPERATION, EOPNOTSUPP);
diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index e3fad80b0795..ff4864d550b8 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -86,7 +86,6 @@ static struct rxrpc_local *rxrpc_alloc_local(const struct sockaddr_rxrpc *srx)
 		atomic_set(&local->usage, 1);
 		INIT_LIST_HEAD(&local->link);
 		INIT_WORK(&local->processor, rxrpc_local_processor);
-		INIT_HLIST_HEAD(&local->services);
 		init_rwsem(&local->defrag_sem);
 		skb_queue_head_init(&local->reject_queue);
 		skb_queue_head_init(&local->event_queue);
@@ -292,7 +291,7 @@ static void rxrpc_local_destroyer(struct rxrpc_local *local)
 	mutex_unlock(&rxrpc_local_mutex);
 
 	ASSERT(RB_EMPTY_ROOT(&local->client_conns));
-	ASSERT(hlist_empty(&local->services));
+	ASSERT(!local->service);
 
 	if (socket) {
 		local->socket = NULL;
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index 82d8134e9287..7d921e56e715 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -131,10 +131,10 @@ int rxrpc_init_server_conn_security(struct rxrpc_connection *conn)
 
 	/* find the service */
 	read_lock(&local->services_lock);
-	hlist_for_each_entry(rx, &local->services, listen_link) {
-		if (rx->srx.srx_service == conn->params.service_id)
-			goto found_service;
-	}
+	rx = rcu_dereference_protected(local->service,
+				       lockdep_is_held(&local->services_lock));
+	if (rx && rx->srx.srx_service == conn->params.service_id)
+		goto found_service;
 
 	/* the service appears to have died */
 	read_unlock(&local->services_lock);

^ permalink raw reply related

* [PATCH net-next 5/6] rxrpc: Request more ACKs in slow-start mode
From: David Howells @ 2016-09-29 22:16 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147518732742.29235.11597637573731081178.stgit@warthog.procyon.org.uk>

Set the request-ACK on more DATA packets whilst we're in slow start mode so
that we get sufficient ACKs back to supply information to configure the
window.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/output.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index ac9a58b619a6..0d47db886f6e 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -284,6 +284,7 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
 	 * ACKs if a DATA packet appears to have been lost.
 	 */
 	if (retrans ||
+	    call->cong_mode == RXRPC_CALL_SLOW_START ||
 	    (call->peer->rtt_usage < 3 && sp->hdr.seq & 1) ||
 	    ktime_before(ktime_add_ms(call->peer->rtt_last_req, 1000),
 			 ktime_get_real()))

^ permalink raw reply related

* [PATCH net-next 6/6] rxrpc: Note serial number being ACK'd in the congestion management trace
From: David Howells @ 2016-09-29 22:16 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147518732742.29235.11597637573731081178.stgit@warthog.procyon.org.uk>

Note the serial number of the packet being ACK'd in the congestion
management trace rather than the serial number of the ACK packet.  Whilst
the serial number of the ACK packet is useful for matching ACK packet in
the output of wireshark, the serial number that the ACK is in response to
is of more use in working out how different trace lines relate.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/input.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 094720dd1eaf..1461d30583c9 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -41,10 +41,10 @@ static void rxrpc_proto_abort(const char *why,
  */
 static void rxrpc_congestion_management(struct rxrpc_call *call,
 					struct sk_buff *skb,
-					struct rxrpc_ack_summary *summary)
+					struct rxrpc_ack_summary *summary,
+					rxrpc_serial_t acked_serial)
 {
 	enum rxrpc_congest_change change = rxrpc_cong_no_change;
-	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 	unsigned int cumulative_acks = call->cong_cumul_acks;
 	unsigned int cwnd = call->cong_cwnd;
 	bool resend = false;
@@ -172,7 +172,7 @@ out_no_clear_ca:
 		cwnd = RXRPC_RXTX_BUFF_SIZE - 1;
 	call->cong_cwnd = cwnd;
 	call->cong_cumul_acks = cumulative_acks;
-	trace_rxrpc_congest(call, summary, sp->hdr.serial, change);
+	trace_rxrpc_congest(call, summary, acked_serial, change);
 	if (resend && !test_and_set_bit(RXRPC_CALL_EV_RESEND, &call->events))
 		rxrpc_queue_call(call);
 	return;
@@ -848,7 +848,7 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 				  false, true,
 				  rxrpc_propose_ack_ping_for_lost_reply);
 
-	return rxrpc_congestion_management(call, skb, &summary);
+	return rxrpc_congestion_management(call, skb, &summary, acked_serial);
 }
 
 /*

^ permalink raw reply related

* Re: next-20160929 build: 2 failures 4 warnings (next-20160929)
From: Arnd Bergmann @ 2016-09-29 22:44 UTC (permalink / raw)
  To: kernel-build-reports
  Cc: Vishwanath Pai, Mark Brown, linaro-kernel, netdev, linux-next,
	linux-arm-kernel, Joshua Hunt, Pablo Neira Ayuso
In-Reply-To: <57ED66CA.3090903@akamai.com>

On Thursday 29 September 2016, Vishwanath Pai wrote:
> I have sent a patch for this a couple of days ago to netdev, it hasn't
> made it to net-next yet. Here's the latest one:
> 
> [PATCH net-next v3] netfilter: xt_hashlimit: Fix link error in 32bit
> arch because of 64bit division
> 
> This should fix the link error.

I also did a patch (not submitted yet), but my solution used 32-bit
math for the version 1 case. I think that would be better so we
don't slow down 32-bit architectures too much (div_u64_u64
is very slow).

	Arnd

^ permalink raw reply

* Re: [PATCH net-next 1/2] openvswitch: mpls: set network header correctly on key extract
From: pravin shelar @ 2016-09-29 23:02 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Linux Kernel Network Developers, David Ahern
In-Reply-To: <170be69b8496bf37e90915e2f05af0464d5f09c6.1475176437.git.jbenc@redhat.com>

On Thu, Sep 29, 2016 at 12:19 PM, Jiri Benc <jbenc@redhat.com> wrote:
> After the 48d2ab609b6b ("net: mpls: Fixups for GSO"), MPLS handling in
> openvswitch was changed to have network header pointing to the start of the
> MPLS headers and inner_network_header pointing after the MPLS headers.
>
> However, key_extract was missed by the mentioned commit, causing incorrect
> headers to be set when a MPLS packet just enters the bridge or after it is
> recirculated.
>
> Fixes: 48d2ab609b6b ("net: mpls: Fixups for GSO")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>

Acked-by: Pravin B Shelar <pshelar@ovn.org>

^ permalink raw reply

* Re: [PATCH net-next 2/2] openvswitch: remove skb_mpls_header
From: pravin shelar @ 2016-09-29 23:03 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Linux Kernel Network Developers, David Ahern
In-Reply-To: <e32cd7d5d03b4cb86a872e76730aa29a2f7d9e38.1475176437.git.jbenc@redhat.com>

On Thu, Sep 29, 2016 at 12:19 PM, Jiri Benc <jbenc@redhat.com> wrote:
> skb_mpls_header is equivalent to skb_network_header now. There's no reason
> to keep it.
>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
> ---
>  include/net/mpls.h        | 11 -----------
>  net/openvswitch/actions.c | 10 +++++-----
>  2 files changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/mpls.h b/include/net/mpls.h
> index 5b3b5addfb08..fde22d0b0ec1 100644
> --- a/include/net/mpls.h
> +++ b/include/net/mpls.h
> @@ -25,15 +25,4 @@ static inline bool eth_p_mpls(__be16 eth_type)
>                 eth_type == htons(ETH_P_MPLS_MC);
>  }
>
> -/*
> - * For non-MPLS skbs this will correspond to the network header.
> - * For MPLS skbs it will be before the network_header as the MPLS
> - * label stack lies between the end of the mac header and the network
> - * header. That is, for MPLS skbs the end of the mac header
> - * is the top of the MPLS label stack.
> - */
> -static inline unsigned char *skb_mpls_header(struct sk_buff *skb)
> -{
> -       return skb_mac_header(skb) + skb->mac_len;
> -}

I think we should keep this API, so that it is clear that MPLS header
mapped skb network header.

^ permalink raw reply

* Re: UDP wierdness around skb_copy_and_csum_datagram_msg()
From: Christian Lamparter @ 2016-09-29 23:28 UTC (permalink / raw)
  To: Jay Smith, Alan Curry; +Cc: Eric Dumazet, netdev, Al Viro, davem@davemloft.net
In-Reply-To: <CAA3=++_6O69hcutQ_FmaivrR3UV-uKScyX8+iHOBV9kraizOFQ@mail.gmail.com>

On Wednesday, September 28, 2016 7:20:39 PM CEST Jay Smith wrote:
> Actually, on a little more searching of this list's archives, I think
> that this discussion:  https://patchwork.kernel.org/patch/9260733/ is
> about exactly the same issue I've found, except from the TCP side. I'm
> cc'ing a few of the participants from that discussion.
> 
> So is the patch proposed there (copying and restoring the entire
> iov_iter in skb_copy_and_csum_datagram_msg()) being considered as a
> fix?

>From Alan's post:

"My ugly patch fixes this in the most obvious way: make a local copy of
msg->msg_iter before the call to skb_copy_and_csum_datagram(), and copy
it back if the checksum is bad, just before goto csum_error;"

IMHO this meant that the patch is a proof of concept for his problem.

> If not, would an alternate one that concealed the save-and-restore logic
> inside iov_iter.c be more acceptable? I'd be happy to produce whatever's
> needed, or yield to someone with stronger feelings about where it should
> go...
Al Viro identified more inconsistencies within the error-paths that deal
with EFAULT in the whole area (in and around skb_copy_and_csum_datagram()).

As far as I can tell the original discussion about the data corruption
issue went off on a tangent and it is stuck in figuring out "How to handle
the errors in tcp_copy_to_iovec()".

As for fixing the issue: I'm happy to test and review patches. 
The trouble is that nobody seem to be able to produce them...

Regards,
Christian

^ permalink raw reply

* Re: UDP wierdness around skb_copy_and_csum_datagram_msg()
From: Eric Dumazet @ 2016-09-30  0:06 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Jay Smith, Alan Curry, netdev, Al Viro, davem@davemloft.net
In-Reply-To: <3897844.9kvF4Wy8aS@debian64>

On Fri, 2016-09-30 at 01:28 +0200, Christian Lamparter wrote:
> On Wednesday, September 28, 2016 7:20:39 PM CEST Jay Smith wrote:
> > Actually, on a little more searching of this list's archives, I think
> > that this discussion:  https://patchwork.kernel.org/patch/9260733/ is
> > about exactly the same issue I've found, except from the TCP side. I'm
> > cc'ing a few of the participants from that discussion.
> > 
> > So is the patch proposed there (copying and restoring the entire
> > iov_iter in skb_copy_and_csum_datagram_msg()) being considered as a
> > fix?
> 
> From Alan's post:
> 
> "My ugly patch fixes this in the most obvious way: make a local copy of
> msg->msg_iter before the call to skb_copy_and_csum_datagram(), and copy
> it back if the checksum is bad, just before goto csum_error;"
> 
> IMHO this meant that the patch is a proof of concept for his problem.
> 
> > If not, would an alternate one that concealed the save-and-restore logic
> > inside iov_iter.c be more acceptable? I'd be happy to produce whatever's
> > needed, or yield to someone with stronger feelings about where it should
> > go...
> Al Viro identified more inconsistencies within the error-paths that deal
> with EFAULT in the whole area (in and around skb_copy_and_csum_datagram()).
> 
> As far as I can tell the original discussion about the data corruption
> issue went off on a tangent and it is stuck in figuring out "How to handle
> the errors in tcp_copy_to_iovec()".
> 
> As for fixing the issue: I'm happy to test and review patches. 
> The trouble is that nobody seem to be able to produce them...
> 

This is doable with a bit of fault injection I believe.

And "ethtool -K eth0 rx off gro off lro off"  to let the TCP receiver
compute the checksum itself.

^ permalink raw reply

* Re: How to submit potential patch in linux kernel
From: Shyam Saini @ 2016-09-30  0:38 UTC (permalink / raw)
  To: Daniel Baluta; +Cc: netdev@vger.kernel.org, Andrew Lunn
In-Reply-To: <CAEnQRZCF8h7+feuDm4Z-HF5fZQ88yiKisg7u0eAEVE27L35y3w@mail.gmail.com>

On Thu, 2016-09-29 at 13:15 +0300, Daniel Baluta wrote:
> Hi Shyam,
> 
> On Thu, Sep 29, 2016 at 1:02 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > 
> > On Thu, Sep 29, 2016 at 04:32:29AM +0530, Shyam Saini wrote:
> > > 
> > > Hi everyone,
> > > 
> > > I'm Shyam, final year undergraduate student. I wanted to know how
> > > one can
> > > submit potential linux kernel patch in networking subsystem.
> > Documentation/SubmittingPatches is a good starting point.
> > 
> > Then post a patch and we will help you learn the rest of the
> > process.
> 
> You can have a look at https://kernelnewbies.org/FirstKernelPatch.
> There is also this presentation from Greg:
> https://www.youtube.com/watch?v=LLBrBBImJt4
> 
> thanks,
> Daniel.


Thanks for your reply. But I already know how to submit patches. I've
submitted some trivial kernel patches. But Now, I want to submit some
potential or good patches. 
So, I wanted to know how can I move to that direction? 

Thanks, 
Shyam

^ permalink raw reply

* Re: [PATCH net-next v3] netfilter: xt_hashlimit: Fix link error in 32bit arch because of 64bit division
From: Maciej Żenczykowski @ 2016-09-30  1:53 UTC (permalink / raw)
  To: Vishwanath Pai
  Cc: Pablo Neira Ayuso, johunt, netfilter-devel, coreteam,
	Linux NetDev, Vishwanath Pai, Patrick McHardy, kadlec, zlpnobody,
	Hannes Frederic Sowa, Eric Dumazet, akpm
In-Reply-To: <20160929173950.GA27689@akamai.com>

> +               return div64_u64((user * HZ * CREDITS_PER_JIFFY_v1),
> +                                XT_HASHLIMIT_SCALE);

spurious ()

> +               return div64_u64((user * HZ * CREDITS_PER_JIFFY),
> +                                XT_HASHLIMIT_SCALE_v2);

spurious ()

but:

Acked-by: Maciej Żenczykowski

^ permalink raw reply

* Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
From: R. Parameswaran @ 2016-09-30  2:39 UTC (permalink / raw)
  To: James Chapman
  Cc: R. Parameswaran, kleptog, netdev, davem, linux-kernel, nprachan,
	rshearma, dfawcus, stephen, acme, lboccass, bhong
In-Reply-To: <57ED30D7.6000009@katalix.com>


Hi James,

On Thu, 29 Sep 2016, James Chapman wrote:

> On 22/09/16 21:52, R. Parameswaran wrote:
> > From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
> > From: "R. Parameswaran" <rparames@brocade.com>
> > Date: Thu, 22 Sep 2016 13:19:25 -0700
> > Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
> >
> > Take into account all of the tunnel encapsulation headers when setting
> > up the MTU on the L2TP logical interface device. Otherwise, packets
> > created by the applications on top of the L2TP layer are larger
> > than they ought to be, relative to the underlay MTU, leading to
> > needless fragmentation once the outer IP encap is added.
> >
> > Specifically, take into account the (outer, underlay) IP header
> > imposed on the encapsulated L2TP packet, and the Layer 2 header
> > imposed on the inner IP packet prior to L2TP encapsulation.
> >
> > Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
> > and the dst entry in the L2TP tunnel socket to directly pull up
> > the underlay MTU (as the baseline number on top of which the
> > encapsulation headers are factored in).  Fall back to Ethernet MTU
> > if this fails.
> >
> > Signed-off-by: R. Parameswaran <rparames@brocade.com>
> >
> > Reviewed-by: "N. Prachanda" <nprachan@brocade.com>,
> > Reviewed-by: "R. Shearman" <rshearma@brocade.com>,
> > Reviewed-by: "D. Fawcus" <dfawcus@brocade.com>
> > ---
> >  net/l2tp/l2tp_eth.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 44 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
> > index 57fc5a4..dbcd6bd 100644
> > --- a/net/l2tp/l2tp_eth.c
> > +++ b/net/l2tp/l2tp_eth.c
> > @@ -30,6 +30,9 @@
> >  #include <net/xfrm.h>
> >  #include <net/net_namespace.h>
> >  #include <net/netns/generic.h>
> > +#include <linux/ip.h>
> > +#include <linux/ipv6.h>
> > +#include <linux/udp.h>
> >  
> >  #include "l2tp_core.h"
> >  
> > @@ -206,6 +209,46 @@ static void l2tp_eth_show(struct seq_file *m, void *arg)
> >  }
> >  #endif
> >  
> > +static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
> > +				struct l2tp_session *session,
> > +				struct net_device *dev)
> > +{
> > +	unsigned int overhead = 0;
> > +	struct dst_entry *dst;
> > +
> > +	if (session->mtu != 0) {
> > +		dev->mtu = session->mtu;
> > +		dev->needed_headroom += session->hdr_len;
> > +		if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
> > +			dev->needed_headroom += sizeof(struct udphdr);
> > +		return;
> > +	}
> > +	overhead = session->hdr_len;
> > +	/* Adjust MTU, factor overhead - underlay L3 hdr, overlay L2 hdr*/
> > +	if (tunnel->sock->sk_family == AF_INET)
> > +		overhead += (ETH_HLEN + sizeof(struct iphdr));
> > +	else if (tunnel->sock->sk_family == AF_INET6)
> > +		overhead += (ETH_HLEN + sizeof(struct ipv6hdr));
> What about options in the IP header? If certain options are set on the
> socket, the IP header may be larger.
> 

Thanks for the reply - It looks like IP options can only be 
enabled through setsockopt on an application's socket (if there's any 
other way to turn on IP options, please let me know - didn't see any 
sysctl setting for transmit). This scenario would come 
into picture when an application opens a raw IP or UDP socket such that it 
routes into the L2TP logical interface.

If you take the case of a plain IP (ethernet) interface, even if an
application opened a socket turning on IP options, it would not change
the MTU of the underlying interface, and it would not affect other 
applications transacting packets on the same interface. I know its not an 
exact parallel to this case, but since the IP option control is per 
application, we probably should not factor it into the L2TP logical interface?
We cannot affect other applications/processes running on the same L2TP 
tunnel. Also, since the application  using IP options knows that it has turned 
on IP options, maybe we can count on it to factor the size of the options 
into the size of the payload it sends into the socket, or set the mtu on the 
L2TP interface through config? 

Other than this, I don't see keepalives or anything else in which the 
kernel will source its own packet into the L2TP interface, outside of 
an application injected packet - if there is something like that, please
let me know. The user space L2TP daemon would probably fall in the 
category of applications.

thanks,

Ramkumar 


> > +	/* Additionally, if the encap is UDP, account for UDP header size */
> > +	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
> > +		overhead += sizeof(struct udphdr);
> > +	/* If PMTU discovery was enabled, use discovered MTU on L2TP device */
> > +	dst = sk_dst_get(tunnel->sock);
> > +	if (dst) {
> > +		u32 pmtu = dst_mtu(dst);
> > +
> > +		if (pmtu != 0)
> > +			dev->mtu = pmtu;
> > +		dst_release(dst);
> > +	}
> > +	/* else (no PMTUD) L2TP dev MTU defaulted to Ethernet MTU in caller */
> > +	session->mtu = dev->mtu - overhead;
> > +	dev->mtu = session->mtu;
> > +	dev->needed_headroom += session->hdr_len;
> > +	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
> > +		dev->needed_headroom += sizeof(struct udphdr);
> > +}
> > +
> >  static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
> >  {
> >  	struct net_device *dev;
> > @@ -255,11 +298,8 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
> >  	}
> >  
> >  	dev_net_set(dev, net);
> > -	if (session->mtu == 0)
> > -		session->mtu = dev->mtu - session->hdr_len;
> > -	dev->mtu = session->mtu;
> > -	dev->needed_headroom += session->hdr_len;
> >  
> > +	l2tp_eth_adjust_mtu(tunnel, session, dev);
> >  	priv = netdev_priv(dev);
> >  	priv->dev = dev;
> >  	priv->session = session;
> 
> 
> 
> 

^ 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