Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2] 3c59x: avoid panic in boomerang_start_xmit when finding page address:
From: David Miller @ 2014-09-04  3:58 UTC (permalink / raw)
  To: nhorman; +Cc: netdev, klassert
In-Reply-To: <1409739445-28021-1-git-send-email-nhorman@tuxdriver.com>

From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed,  3 Sep 2014 06:17:25 -0400

> This bug was reported on a very old kernel (RHEL6, 2.6.32-491.el6):
...
> v2) Converted PCI_DMA_TODEVICE to DMA_TO_DEVICE.  Thanks Dave!

Ummm, it's still PCI_DMA_TODEVICE.

> -					cpu_to_le32(pci_map_single(
> -						VORTEX_PCI(vp),
> -						(void *)skb_frag_address(frag),
> -						skb_frag_size(frag), PCI_DMA_TODEVICE));
> +					cpu_to_le32(skb_frag_dma_map(
> +						&VORTEX_PCI(vp)->dev,
> +						frag,
> +						frag->page_offset, frag->size, PCI_DMA_TODEVICE));

^ permalink raw reply

* Re: [PATCH net-next] mlx4_en: Convert the normal skb free path to dev_consume_skb_any()
From: David Miller @ 2014-09-04  4:00 UTC (permalink / raw)
  To: raj; +Cc: netdev, amirv
In-Reply-To: <20140903161800.301F629002CD@tardy>

From: raj@tardy.usa.hp.com (Rick Jones)
Date: Wed,  3 Sep 2014 09:18:00 -0700 (PDT)

> From: Rick Jones <rick.jones2@hp.com>
> 
> It would appear the mlx4_en driver was still making a call to
> dev_kfree_skb_any() where dev_consume_skb_any() would be more
> appropriate.  This should make dropped packet profiling/tracking
> easier/better over a NIC driven by mlx4_en.
> 
> Signed-off-by: Rick Jones <rick.jones2@hp.com>

Applied, thank you.

^ permalink raw reply

* Re: [Patch net-next 04/11] net:fec: add multiqueue support
From: Florian Fainelli @ 2014-09-04  4:09 UTC (permalink / raw)
  To: Zhi Li
  Cc: Frank Li, Duan Fugang-B38611, David Miller,
	netdev@vger.kernel.org, Shawn Guo,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <CAHrpEqRSGg9=B8nk-PFkayP1kN8eQ5UnBvjikR5svub-wD-d6w@mail.gmail.com>

2014-09-03 20:41 GMT-07:00 Zhi Li <lznuaa@gmail.com>:
> On Wed, Sep 3, 2014 at 4:27 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 09/03/2014 01:38 PM, Frank Li wrote:
>>> From: Fugang Duan <B38611@freescale.com>
>>>
>>> Add multiqueue support, which is compatible with previous single queue
>>> driver for FEC/ENET IPs.
>>>
>>> By default, the tx/rx queue number is 1, user can config the queue number
>>> at DTS file like this:
>>>       fsl,num_tx_queues=<3>;
>>>       fsl,num_rx_queues=<3>;
>>>
>>> Ethernet multiqueue mechanism can improve performance in SMP system.
>>> For single hw queue, multiqueue can balance cpu loading.
>>> For multi hw queues, multiple cores can process network packets in parallel,
>>> and refer the article for the detail advantage for multiqueue:
>>> http://vger.kernel.org/~davem/davem_nyc09.pdf
>>
>> You should probably split that patch in a minimum of two, or more
>> different parts.
>>
>> Two parts:
>>
>> - add TX multiqueue support
>> - add RX multiqueue support
>>
>
> I understood your concern.
> We also need consider logical integration.

Sure, hence the reason why I suggest you make it happen in different
parts, what you can do is make sure that you parse "fsl,num_tx_queues"
and "fsl,num_rx_queues" as one of the last patches in this patch
series. That way, we achieve a logical breakdown of the changes and
only support and enable multiqueue when the driver is ready.

>
> Separate this patch just reduce changed lines in a patch.

That is exactly my point, right now there is too much churn to make a
reviewer able to spot potential problems.

> Most of this patch change is from fep-> to txq-> or rxq->
>
> best regards
> Frank Li
>
>> More parts:
>>
>> - prepare for TX multiqueue (introduce queue structure, definitions)
>> - transition TX path to use queue 0
>> - add support for the two other TX queues
>> - prepare for RX multiqueue (introduce queue structure, definitions)
>> - prepare for multiqueue, hardcode RX path to use queue 0
>> - add support for the two other RX queues
>>
>> The number of changes are extremely hard to review...
>> --
>> Florian



-- 
Florian

^ permalink raw reply

* [PATCH net-next] ipv4: fix a race in update_or_create_fnhe()
From: Eric Dumazet @ 2014-09-04  5:21 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

nh_exceptions is effectively used under rcu, but lacks proper
barriers. Between kzalloc() and setting of nh->nh_exceptions(),
we need a proper memory barrier.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: 4895c771c7f00 ("ipv4: Add FIB nexthop exceptions.")
---
 include/net/ip_fib.h     |    2 +-
 net/ipv4/fib_semantics.c |    8 +++++---
 net/ipv4/route.c         |    6 +++---
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 9922093f575e..f30fd554127e 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -87,7 +87,7 @@ struct fib_nh {
 	int			nh_saddr_genid;
 	struct rtable __rcu * __percpu *nh_pcpu_rth_output;
 	struct rtable __rcu	*nh_rth_input;
-	struct fnhe_hash_bucket	*nh_exceptions;
+	struct fnhe_hash_bucket	__rcu *nh_exceptions;
 };
 
 /*
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index b10cd43a4722..5b6efb3d2308 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -157,9 +157,12 @@ static void rt_fibinfo_free(struct rtable __rcu **rtp)
 
 static void free_nh_exceptions(struct fib_nh *nh)
 {
-	struct fnhe_hash_bucket *hash = nh->nh_exceptions;
+	struct fnhe_hash_bucket *hash;
 	int i;
 
+	hash = rcu_dereference_protected(nh->nh_exceptions, 1);
+	if (!hash)
+		return;
 	for (i = 0; i < FNHE_HASH_SIZE; i++) {
 		struct fib_nh_exception *fnhe;
 
@@ -205,8 +208,7 @@ static void free_fib_info_rcu(struct rcu_head *head)
 	change_nexthops(fi) {
 		if (nexthop_nh->nh_dev)
 			dev_put(nexthop_nh->nh_dev);
-		if (nexthop_nh->nh_exceptions)
-			free_nh_exceptions(nexthop_nh);
+		free_nh_exceptions(nexthop_nh);
 		rt_fibinfo_free_cpus(nexthop_nh->nh_pcpu_rth_output);
 		rt_fibinfo_free(&nexthop_nh->nh_rth_input);
 	} endfor_nexthops(fi);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index eaa4b000c7b4..44b0cbdd76f1 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -628,12 +628,12 @@ static void update_or_create_fnhe(struct fib_nh *nh, __be32 daddr, __be32 gw,
 
 	spin_lock_bh(&fnhe_lock);
 
-	hash = nh->nh_exceptions;
+	hash = rcu_dereference(nh->nh_exceptions);
 	if (!hash) {
 		hash = kzalloc(FNHE_HASH_SIZE * sizeof(*hash), GFP_ATOMIC);
 		if (!hash)
 			goto out_unlock;
-		nh->nh_exceptions = hash;
+		rcu_assign_pointer(nh->nh_exceptions, hash);
 	}
 
 	hash += hval;
@@ -1242,7 +1242,7 @@ static unsigned int ipv4_mtu(const struct dst_entry *dst)
 
 static struct fib_nh_exception *find_exception(struct fib_nh *nh, __be32 daddr)
 {
-	struct fnhe_hash_bucket *hash = nh->nh_exceptions;
+	struct fnhe_hash_bucket *hash = rcu_dereference(nh->nh_exceptions);
 	struct fib_nh_exception *fnhe;
 	u32 hval;
 

^ permalink raw reply related

* Re: [net-next PATCH] qdisc: exit case fixes for skb list handling in qdisc layer
From: Eric Dumazet @ 2014-09-04  5:24 UTC (permalink / raw)
  To: David Miller; +Cc: brouer, netdev, fw, hannes, dborkman
In-Reply-To: <20140903.204256.1398300065789241714.davem@davemloft.net>

On Wed, 2014-09-03 at 20:42 -0700, David Miller wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Wed, 03 Sep 2014 12:12:50 +0200
> 
> > More minor fixes to merge commit 53fda7f7f9e (Merge branch 'xmit_list')
> > that allows us to work with a list of SKBs.
> > 
> > Fixing exit cases in qdisc_reset() and qdisc_destroy(), where a
> > leftover requeued SKB (qdisc->gso_skb) can have the potential of
> > being a skb list, thus use kfree_skb_list().
> > 
> > This is a followup to commit 10770bc2d1 ("qdisc: adjustments for
> > API allowing skb list xmits").
> > 
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> 
> Applied.

Strange, I sent same patch a little earlier ;)

^ permalink raw reply

* Re: [PATCH] iproute2: Refactor: using new nl_msg struct for the all Netlink handlers
From: Vadim Kochan @ 2014-09-04  5:39 UTC (permalink / raw)
  To: Stephen Hemminger, netdev
In-Reply-To: <CAMw6YJJHRet+Z_855OEpDaAYfiVUgORo8T0p_XF2p-WN6fiYdg@mail.gmail.com>

+netdev@vger.kernel.org

On Thu, Sep 4, 2014 at 8:33 AM, Vadim Kochan <vadim4j@gmail.com> wrote:
> I think that even if convert iproute2 to some other lib it would be
> good to have some custome struct which
> would be used by handlers.
>
> On Thu, Sep 4, 2014 at 4:33 AM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>> On Wed,  3 Sep 2014 23:47:04 +0300
>> Vadim Kochan <vadim4j@gmail.com> wrote:
>>
>>> It will easy to extend with new parameters in future.
>>>
>>> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
>>
>> Rather than go to all this effort, lets just convert iproute2 to use
>> a sane netlink library like libmnl.
>>

^ permalink raw reply

* Re: [net-next PATCH] qdisc: exit case fixes for skb list handling in qdisc layer
From: Jesper Dangaard Brouer @ 2014-09-04  5:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, fw, hannes, dborkman, brouer
In-Reply-To: <1409808259.26422.84.camel@edumazet-glaptop2.roam.corp.google.com>

On Wed, 03 Sep 2014 22:24:19 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Wed, 2014-09-03 at 20:42 -0700, David Miller wrote:
> > From: Jesper Dangaard Brouer <brouer@redhat.com>
> > Date: Wed, 03 Sep 2014 12:12:50 +0200
> > 
> > > More minor fixes to merge commit 53fda7f7f9e (Merge branch 'xmit_list')
> > > that allows us to work with a list of SKBs.
> > > 
> > > Fixing exit cases in qdisc_reset() and qdisc_destroy(), where a
> > > leftover requeued SKB (qdisc->gso_skb) can have the potential of
> > > being a skb list, thus use kfree_skb_list().
> > > 
> > > This is a followup to commit 10770bc2d1 ("qdisc: adjustments for
> > > API allowing skb list xmits").
> > > 
> > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > 
> > Applied.
> 
> Strange, I sent same patch a little earlier ;)

Yes, very strange, especially as Dave also said he had applied your
patch too (I did comment on your post, apologizing for not noticing
your patch, before sending this one).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-04  5:41 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
	Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
	Peter Zijlstra, H. Peter Anvin, Andrew Morton, Kees Cook,
	Linux API, Network Development, LKML, Daniel Borkmann

On Wed, Sep 3, 2014 at 8:46 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/03/2014 05:17 AM, Alexei Starovoitov wrote:
>>
>> allow user space to generate eBPF programs
>>
>> uapi/linux/bpf.h: eBPF instruction set definition
>>
>> linux/filter.h: the rest
>>
>> This patch only moves macro definitions, but practically it freezes
>> existing
>> eBPF instruction set, though new instructions can still be added in the
>> future.
>>
>> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
>> may conflict with existing applications.
>>
>> Full eBPF ISA description is in Documentation/networking/filter.txt
>>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>
>
> Ok, given you post the remaining two RFCs later on this window as
> you indicate, I have no objections:
>
> Acked-by: Daniel Borkmann <dborkman@redhat.com>

David,

I see that all 4 patches are marked as rfc, whereas I tagged
only 3 and 4 as 'rfc' and mentioned in cover letter that they
will be coming for real in stage III. Here they support patch 2
and show the future changes to bpf.h file.
Patches 1 and 2 are good to go.
Note that they're on top of Hannes's patch:
http://patchwork.ozlabs.org/patch/385266/
Do you want me to resubmit just first two?

Thanks

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Eliezer Tamir @ 2014-09-04  6:51 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jason Wang, Ingo Molnar, Mike Galbraith, davem, netdev,
	linux-kernel, Peter Zijlstra, Ingo Molnar
In-Reply-To: <20140903075104.GA6187@redhat.com>

On 03/09/2014 10:51, Michael S. Tsirkin wrote:
> On Wed, Sep 03, 2014 at 09:49:10AM +0300, Eliezer Tamir wrote:
>> On 02/09/2014 11:31, Michael S. Tsirkin wrote:
>>> On Tue, Sep 02, 2014 at 09:15:18AM +0300, Eliezer Tamir wrote:

>> Busy polling is not a general purpose feature, it's not something you
>> can casually turn on and will "just work". Most applications should not
>> be using busy polling. Currently it is used by multiserver applications
>> that you spend days tuning to specific platforms.
>>
>> What the user wants is to lower both avg and maximum latencies, at the
>> expense of everything else including power efficiency and sometimes
>> even throughput. The only exception is making the system crash ;)
>>
>> While letting other things take precedence over busy polling might not
>> hurt the avg latency much, it will kill your maximum latency.
> 
> If scheduler happens to run both server and client on the
> same CPU, polling will hurt maximum latency even more.
> So I guess different users want different things.
> 
> How about applications giving us a hint what they prefer?
> For example, a new flag that says "I don't have anything useful to do so
> let's do busy polling but my server is on the local system, so please
> only poll if CPU is otherwise idle".

I'm sorry for being ambiguous, when I said multi-server application, I
meant an app that runs on more than one server machine.

The loopback test is as far as I know not interesting.

Of course if busypoll becomes interesting for virtualization over
loopback, I have no problem with that, provided that there is a way to
get the old behavior and that it is well documented.

-Eliezer

^ permalink raw reply

* [PATCH] net: ethernet: cpsw: improve interrupt lookup logic in cpsw_probe()
From: Daniel Mack @ 2014-09-04  7:00 UTC (permalink / raw)
  To: davem; +Cc: julia.lawall, netdev, mugunthanvnm, george.cherian, Daniel Mack

From: Daniel Mack <zonque@gmail.com>

Simplify the interrupt resource lookup code in cpsw_probe() by the
following:

 * Only look at the first member of the resource. As the driver only
   works for DT-enabled platforms anyway, a resource of type
   IORESOURCE_IRQ will only contain one single entry
   (res->start == res->end), so there is no need for the iteration.

 * Add a bounds check to avoid overflows if we are passed more than
   ARRAY_SIZE(priv->irqs_table) resources.

 * Assign 'ret' with the return value of devm_request_irq() so that
   cpsw_probe() returns the appropriate error code.

 * If devm_request_irq() fails, report the error code in the log
   message.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/net/ethernet/ti/cpsw.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 999fb72..03b4099 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2232,18 +2232,24 @@ static int cpsw_probe(struct platform_device *pdev)
 	}
 
 	while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k))) {
-		for (i = res->start; i <= res->end; i++) {
-			if (devm_request_irq(&pdev->dev, i, cpsw_interrupt, 0,
-					     dev_name(&pdev->dev), priv)) {
-				dev_err(priv->dev, "error attaching irq\n");
-				goto clean_ale_ret;
-			}
-			priv->irqs_table[k] = i;
-			priv->num_irqs = k + 1;
+		if (k >= ARRAY_SIZE(priv->irqs_table)) {
+			ret = -EINVAL;
+			goto clean_ale_ret;
 		}
+
+		ret = devm_request_irq(&pdev->dev, res->start, cpsw_interrupt,
+				       0, dev_name(&pdev->dev), priv);
+		if (ret < 0) {
+			dev_err(priv->dev, "error attaching irq (%d)\n", ret);
+			goto clean_ale_ret;
+		}
+
+		priv->irqs_table[k] = res->start;
 		k++;
 	}
 
+	priv->num_irqs = k;
+
 	ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
 
 	ndev->netdev_ops = &cpsw_netdev_ops;
-- 
2.1.0

^ permalink raw reply related

* [patch -next] hyperv: NULL dereference on error
From: Dan Carpenter @ 2014-09-04 11:11 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang; +Cc: devel, kernel-janitors, netdev

We try to call free_netvsc_device(net_device) when "net_device" is NULL.
It leads to an Oops.

Fixes: f90251c8a6d0 ('hyperv: Increase the buffer length for netvsc_channel_cb()')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 5b5644a..977984b 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1054,10 +1054,8 @@ int netvsc_device_add(struct hv_device *device, void *additional_info)
 	struct net_device *ndev;
 
 	net_device = alloc_net_device(device);
-	if (!net_device) {
-		ret = -ENOMEM;
-		goto cleanup;
-	}
+	if (!net_device)
+		return -ENOMEM;
 
 	net_device->ring_size = ring_size;

^ permalink raw reply related

* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Hannes Frederic Sowa @ 2014-09-04 11:50 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: Alexei Starovoitov, Eric Dumazet, Daniel Borkmann, Eric Dumazet,
	Denis Kirjanov, netdev@vger.kernel.org, Markos Chandras,
	Martin Schwidefsky
In-Reply-To: <CAHj3AVn8q-JS0Eq0QrfpBo-WfJ+-wXumhNoeDAdkPZC7LSeB5g@mail.gmail.com>

On Do, 2014-09-04 at 10:09 +0400, Denis Kirjanov wrote:
> On Thursday, September 4, 2014, Alexei Starovoitov <
> alexei.starovoitov@gmail.com> wrote:
> 
> > On Wed, Sep 3, 2014 at 7:05 PM, Eric Dumazet <eric.dumazet@gmail.com
> > <javascript:;>> wrote:
> > > On Thu, 2014-09-04 at 03:25 +0200, Hannes Frederic Sowa wrote:
> > >
> > >> Can't we add an address marker to struct sk_buff?
> > >>
> > >> Several possibilities are available:
> > >>
> > >> ptrdiff_t pkt_type_offset[0]  before the pkt_type flags field
> > >>
> > >> If one wants to make it more expressive:
> > >> typedef struct {} mark_struct_offset;
> > >> and add
> > >> mark_struct_offset pkt_type_offset;
> > >> at appropriate places
> > >>
> > >> Or maybe an anonymous union?
> > >>
> > >> pkt_type_offset would become a simple offsetof(struct sk_buff,
> > >> pkt_type_offset) then and there is no need for BUG_ON then.
> > >
> > >
> > > You can try, and make sure this works on all gcc compilers, even the one
> > > Andrew Morton uses.
> > >
> > > And of course, you need to not introduce holes doing so.

Sure, it will work as expected. kmemleak uses the field[0] technique and
is already used in struct sk_buff.

I checked below patch with pahole and it doesn't change anything in size
or fieldoffsets.

> >
> > good points.
> > I think Hannes's idea is the best.
> 
> Agreed.
> 
> > It will help to get rid of helper altogether.
> > For my bpf syscall proposal I've used anonymous unions and tested
> > them with gcc 4.2 - 4.9 on x64/i386/arm32/sparc64 and clang.
> > All compilers were doing just fine. So it should work.
> 
> Nice. Alexei, care to send a patch then?

Denis, I thought about something like this, you can incorperate this in
your patch if you can give it a test and check for other architectures,
thanks!

diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
index 05a5661..0fd3f95 100644
--- a/arch/mips/net/bpf_jit.c
+++ b/arch/mips/net/bpf_jit.c
@@ -765,27 +765,6 @@ static u64 jit_get_skb_w(struct sk_buff *skb,
unsigned offset)
 	return (u64)err << 32 | ntohl(ret);
 }
 
-#ifdef __BIG_ENDIAN_BITFIELD
-#define PKT_TYPE_MAX	(7 << 5)
-#else
-#define PKT_TYPE_MAX	7
-#endif
-static int pkt_type_offset(void)
-{
-	struct sk_buff skb_probe = {
-		.pkt_type = ~0,
-	};
-	u8 *ct = (u8 *)&skb_probe;
-	unsigned int off;
-
-	for (off = 0; off < sizeof(struct sk_buff); off++) {
-		if (ct[off] == PKT_TYPE_MAX)
-			return off;
-	}
-	pr_err_once("Please fix pkt_type_offset(), as pkt_type couldn't be
found\n");
-	return -1;
-}
-
 static int build_body(struct jit_ctx *ctx)
 {
 	void *load_func[] = {jit_get_skb_b, jit_get_skb_h, jit_get_skb_w};
@@ -1332,11 +1311,7 @@ jmp_cmp:
 		case BPF_ANC | SKF_AD_PKTTYPE:
 			ctx->flags |= SEEN_SKB;
 
-			off = pkt_type_offset();
-
-			if (off < 0)
-				return -1;
-			emit_load_byte(r_tmp, r_skb, off, ctx);
+			emit_load_byte(r_tmp, r_skb, skb_pkt_type_offset(), ctx);
 			/* Keep only the last 3 bits */
 			emit_andi(r_A, r_tmp, PKT_TYPE_MAX, ctx);
 #ifdef __BIG_ENDIAN_BITFIELD
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 61e45b7..8039bb8 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -223,37 +223,6 @@ static void bpf_jit_epilogue(struct bpf_jit *jit)
 	EMIT2(0x07fe);
 }
 
-/* Helper to find the offset of pkt_type in sk_buff
- * Make sure its still a 3bit field starting at the MSBs within a byte.
- */
-#define PKT_TYPE_MAX 0xe0
-static int pkt_type_offset;
-
-static int __init bpf_pkt_type_offset_init(void)
-{
-	struct sk_buff skb_probe = {
-		.pkt_type = ~0,
-	};
-	char *ct = (char *)&skb_probe;
-	int off;
-
-	pkt_type_offset = -1;
-	for (off = 0; off < sizeof(struct sk_buff); off++) {
-		if (!ct[off])
-			continue;
-		if (ct[off] == PKT_TYPE_MAX)
-			pkt_type_offset = off;
-		else {
-			/* Found non matching bit pattern, fix needed. */
-			WARN_ON_ONCE(1);
-			pkt_type_offset = -1;
-			return -1;
-		}
-	}
-	return 0;
-}
-device_initcall(bpf_pkt_type_offset_init);
-
 /*
  * make sure we dont leak kernel information to user
  */
@@ -753,12 +722,10 @@ call_fn:	/* lg %r1,<d(function)>(%r13) */
 		}
 		break;
 	case BPF_ANC | SKF_AD_PKTTYPE:
-		if (pkt_type_offset < 0)
-			goto out;
 		/* lhi %r5,0 */
 		EMIT4(0xa7580000);
 		/* ic %r5,<d(pkt_type_offset)>(%r2) */
-		EMIT4_DISP(0x43502000, pkt_type_offset);
+		EMIT4_DISP(0x43502000, skb_pkt_type_offset());
 		/* srl %r5,5 */
 		EMIT4_DISP(0x88500000, 5);
 		break;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 02529fc..87b86aa 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -548,6 +548,10 @@ struct sk_buff {
 				ip_summed:2,
 				nohdr:1,
 				nfctinfo:3;
+	/* __pkt_type_offset marks the offset of the bitfield pkt_type
+	 * contains, so bpf can relatively address it
+	 */
+	int			__pkt_type_offset[0];
 	__u8			pkt_type:3,
 				fclone:2,
 				ipvs_property:1,
@@ -647,6 +651,17 @@ struct sk_buff {
 #define SKB_ALLOC_FCLONE	0x01
 #define SKB_ALLOC_RX		0x02
 
+#ifdef __BIG_ENDIAN_BITFIELD
+#define PKT_TYPE_MAX   (7 << 5)
+#else
+#define PKT_TYPE_MAX   7
+#endif
+
+static inline size_t skb_pkt_type_offset(void)
+{
+	return offsetof(struct sk_buff, __pkt_type_offset);
+}
+
 /* Returns true if the skb was allocated from PFMEMALLOC reserves */
 static inline bool skb_pfmemalloc(const struct sk_buff *skb)
 {
diff --git a/net/core/filter.c b/net/core/filter.c
index d814b8a..ca64d7a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -87,30 +87,6 @@ int sk_filter(struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(sk_filter);
 
-/* Helper to find the offset of pkt_type in sk_buff structure. We want
- * to make sure its still a 3bit field starting at a byte boundary;
- * taken from arch/x86/net/bpf_jit_comp.c.
- */
-#ifdef __BIG_ENDIAN_BITFIELD
-#define PKT_TYPE_MAX	(7 << 5)
-#else
-#define PKT_TYPE_MAX	7
-#endif
-static unsigned int pkt_type_offset(void)
-{
-	struct sk_buff skb_probe = { .pkt_type = ~0, };
-	u8 *ct = (u8 *) &skb_probe;
-	unsigned int off;
-
-	for (off = 0; off < sizeof(struct sk_buff); off++) {
-		if (ct[off] == PKT_TYPE_MAX)
-			return off;
-	}
-
-	pr_err_once("Please fix %s, as pkt_type couldn't be found!\n",
__func__);
-	return -1;
-}
-
 static u64 __skb_get_pay_offset(u64 ctx, u64 a, u64 x, u64 r4, u64 r5)
 {
 	return __skb_get_poff((struct sk_buff *)(unsigned long) ctx);
@@ -191,7 +167,7 @@ static bool convert_bpf_extensions(struct
sock_filter *fp,
 
 	case SKF_AD_OFF + SKF_AD_PKTTYPE:
 		*insn = BPF_LDX_MEM(BPF_B, BPF_REG_A, BPF_REG_CTX,
-				    pkt_type_offset());
+				    skb_pkt_type_offset());
 		if (insn->off < 0)
 			return false;
 		insn++;

^ permalink raw reply related

* Re: [PATCH v5 1/4] ethernet: arc: Add support for Rockchip SoC layer device tree bindings
From: Heiko Stübner @ 2014-09-04 12:05 UTC (permalink / raw)
  To: Romain Perier; +Cc: linux-rockchip, devicetree, arnd, linux-arm-kernel, netdev
In-Reply-To: <1409763165-16461-1-git-send-email-romain.perier@gmail.com>

Am Mittwoch, 3. September 2014, 16:52:42 schrieb Romain Perier:
> This patch defines a platform glue layer for Rockchip SoCs which
> support arc-emac driver. It ensures that regulator for the rmii is on
> before trying to connect to the ethernet controller. It applies right
> speed and mode changes to the grf when ethernet settings change.
> 
> Signed-off-by: Romain Perier <romain.perier@gmail.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/net/ethernet/arc/Kconfig         |  10 ++
>  drivers/net/ethernet/arc/Makefile        |   1 +
>  drivers/net/ethernet/arc/emac.h          |   4 +-
>  drivers/net/ethernet/arc/emac_main.c     |   2 +
>  drivers/net/ethernet/arc/emac_rockchip.c | 228
> +++++++++++++++++++++++++++++++ 5 files changed, 244 insertions(+), 1
> deletion(-)
>  create mode 100644 drivers/net/ethernet/arc/emac_rockchip.c
> 
> diff --git a/drivers/net/ethernet/arc/Kconfig
> b/drivers/net/ethernet/arc/Kconfig index 89e04fd..8e262e2 100644
> --- a/drivers/net/ethernet/arc/Kconfig
> +++ b/drivers/net/ethernet/arc/Kconfig
> @@ -32,4 +32,14 @@ config ARC_EMAC
>  	  non-standard on-chip ethernet device ARC EMAC 10/100 is used.
>  	  Say Y here if you have such a board.  If unsure, say N.
> 
> +config EMAC_ROCKCHIP
> +	tristate "Rockchip EMAC support"
> +	select ARC_EMAC_CORE
> +	depends on OF_IRQ && OF_NET && REGULATOR
> +	---help---
> +	  Support for Rockchip RK3066/RK3188 EMAC ethernet controllers.
> +	  This selects Rockchip SoC glue layer support for the
> +	  emac device driver. This driver is used for RK3066/RK3188
> +	  EMAC ethernet controller.
> +
>  endif # NET_VENDOR_ARC
> diff --git a/drivers/net/ethernet/arc/Makefile
> b/drivers/net/ethernet/arc/Makefile index 241bb80..79108af 100644
> --- a/drivers/net/ethernet/arc/Makefile
> +++ b/drivers/net/ethernet/arc/Makefile
> @@ -5,3 +5,4 @@
>  arc_emac-objs := emac_main.o emac_mdio.o
>  obj-$(CONFIG_ARC_EMAC_CORE) += arc_emac.o
>  obj-$(CONFIG_ARC_EMAC) += emac_arc.o
> +obj-$(CONFIG_EMAC_ROCKCHIP) += emac_rockchip.o
> diff --git a/drivers/net/ethernet/arc/emac.h
> b/drivers/net/ethernet/arc/emac.h index eb2ba67..dae1ac3 100644
> --- a/drivers/net/ethernet/arc/emac.h
> +++ b/drivers/net/ethernet/arc/emac.h
> @@ -123,9 +123,11 @@ struct buffer_state {
>   * @speed:	PHY's last set speed.
>   */
>  struct arc_emac_priv {
> -	/* Devices */
>  	const char *drv_name;
>  	const char *drv_version;
> +	void (*set_mac_speed)(void *priv, unsigned int speed);
> +
> +	/* Devices */
>  	struct device *dev;
>  	struct phy_device *phy_dev;
>  	struct mii_bus *bus;
> diff --git a/drivers/net/ethernet/arc/emac_main.c
> b/drivers/net/ethernet/arc/emac_main.c index b35c69e..a08f343 100644
> --- a/drivers/net/ethernet/arc/emac_main.c
> +++ b/drivers/net/ethernet/arc/emac_main.c
> @@ -48,6 +48,8 @@ static void arc_emac_adjust_link(struct net_device *ndev)
>  	if (priv->speed != phy_dev->speed) {
>  		priv->speed = phy_dev->speed;
>  		state_changed = 1;
> +		if (priv->set_mac_speed)
> +			priv->set_mac_speed(priv, priv->speed);
>  	}
> 
>  	if (priv->duplex != phy_dev->duplex) {
> diff --git a/drivers/net/ethernet/arc/emac_rockchip.c
> b/drivers/net/ethernet/arc/emac_rockchip.c new file mode 100644
> index 0000000..51d0585
> --- /dev/null
> +++ b/drivers/net/ethernet/arc/emac_rockchip.c
> @@ -0,0 +1,228 @@
> +/**
> + * emac-rockchip.c - Rockchip EMAC specific glue layer
> + *
> + * Copyright (C) 2014 Romain Perier <romain.perier@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/etherdevice.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_net.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include "emac.h"
> +
> +#define DRV_NAME        "rockchip_emac"
> +#define DRV_VERSION     "1.0"
> +
> +#define GRF_MODE_MII		(1UL << 0)
> +#define GRF_MODE_RMII		(0UL << 0)
> +#define GRF_SPEED_10M		(0UL << 1)
> +#define GRF_SPEED_100M		(1UL << 1)
> +#define GRF_SPEED_ENABLE_BIT	(1UL << 17)
> +#define GRF_MODE_ENABLE_BIT	(1UL << 16)
> +
> +struct emac_rockchip_soc_data {
> +	int grf_offset;
> +};
> +
> +struct rockchip_priv_data {
> +	struct arc_emac_priv emac;
> +	struct regmap *grf;
> +	const struct emac_rockchip_soc_data *soc_data;
> +	struct regulator *regulator;
> +	struct clk *refclk;
> +};
> +
> +static void emac_rockchip_set_mac_speed(void *priv, unsigned int speed)
> +{
> +	struct rockchip_priv_data *emac = priv;
> +	u32 data;
> +	int err = 0;
> +
> +	/* write-enable bits */
> +	data = GRF_SPEED_ENABLE_BIT;
> +
> +	switch(speed) {
> +	case 10:
> +		data |= GRF_SPEED_10M;
> +		break;
> +	case 100:
> +		data |= GRF_SPEED_100M;
> +		break;
> +	default:
> +		pr_err("speed %u not supported\n", speed);
> +		return;
> +	}
> +
> +	err = regmap_write(emac->grf, emac->soc_data->grf_offset, data);
> +	if (err)
> +		pr_err("unable to apply speed %u to grf (%d)\n", speed, err);
> +}
> +
> +static const struct emac_rockchip_soc_data emac_rockchip_dt_data[] = {
> +	{ .grf_offset = 0x154 }, /* rk3066 */
> +	{ .grf_offset = 0x0a4 }, /* rk3188 */
> +};
> +
> +static const struct of_device_id emac_rockchip_dt_ids[] = {
> +	{ .compatible = "rockchip,rk3066-emac", .data = &emac_rockchip_dt_data[0]
> }, +	{ .compatible = "rockchip,rk3188-emac", .data =
> &emac_rockchip_dt_data[1] }, +	{ /* Sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, emac_rockchip_dt_ids);
> +
> +static int emac_rockchip_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct net_device *ndev;
> +	struct rockchip_priv_data *priv;
> +	const struct of_device_id *match;
> +	u32 data;
> +	int err, interface;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	ndev = alloc_etherdev(sizeof(struct rockchip_priv_data));
> +	if (!ndev)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, ndev);
> +	SET_NETDEV_DEV(ndev, dev);
> +
> +	priv = netdev_priv(ndev);
> +	priv->emac.drv_name = DRV_NAME;
> +	priv->emac.drv_version = DRV_VERSION;
> +	priv->emac.set_mac_speed = emac_rockchip_set_mac_speed;
> +
> +	interface = of_get_phy_mode(dev->of_node);
> +
> +	/* RK3066 and RK3188 SoCs only support RMII */
> +	if (interface != PHY_INTERFACE_MODE_RMII) {
> +		dev_err(dev, "unsupported phy interface mode %d\n", interface);
> +		err = -ENOTSUPP;
> +		goto out_netdev;
> +	}
> +
> +	priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
> +	if (IS_ERR(priv->grf)) {
> +		dev_err(dev, "failed to retrieve global register file (%ld)\n",
> PTR_ERR(priv->grf)); +		err = PTR_ERR(priv->grf);
> +		goto out_netdev;
> +	}
> +
> +	match = of_match_node(emac_rockchip_dt_ids, dev->of_node);
> +	priv->soc_data = match->data;
> +
> +	priv->emac.clk = devm_clk_get(dev, "hclk");
> +	if (IS_ERR(priv->emac.clk)) {
> +		dev_err(dev, "failed to retrieve host clock (%ld)\n",
> PTR_ERR(priv->emac.clk)); +		err = PTR_ERR(priv->emac.clk);
> +		goto out_netdev;
> +	}
> +
> +	priv->refclk = devm_clk_get(dev, "macref");
> +	if (IS_ERR(priv->refclk)) {
> +		dev_err(dev, "failed to retrieve reference clock (%ld)\n",
> PTR_ERR(priv->refclk)); +		err = PTR_ERR(priv->refclk);
> +		goto out_netdev;
> +	}
> +
> +	err = clk_prepare_enable(priv->refclk);
> +	if (err) {
> +		dev_err(dev, "failed to enable reference clock (%d)\n", err);
> +		goto out_netdev;
> +	}
> +
> +	/* Optional regulator for PHY */
> +	priv->regulator = devm_regulator_get_optional(dev, "phy");
> +	if (IS_ERR(priv->regulator)) {
> +		if (PTR_ERR(priv->regulator) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		dev_err(dev, "no regulator found\n");
> +		priv->regulator = NULL;
> +	}
> +
> +	if (priv->regulator) {
> +		err = regulator_enable(priv->regulator);
> +		if (err) {
> +			dev_err(dev, "failed to enable phy-supply (%d)\n", err);
> +			goto out_clk_disable;
> +		}
> +	}
> +
> +	err = arc_emac_probe(ndev, interface);
> +	if (err)
> +		goto out_regulator_disable;
> +
> +	/* write-enable bits */
> +	data = GRF_MODE_ENABLE_BIT | GRF_SPEED_ENABLE_BIT;
> +
> +	data |= GRF_SPEED_100M;
> +	data |= GRF_MODE_RMII;
> +
> +	err = regmap_write(priv->grf, priv->soc_data->grf_offset, data);
> +	if (err) {
> +		dev_err(dev, "unable to apply initial settings to grf (%d)\n", err);
> +		goto out_regulator_disable;
> +	}
> +
> +	/* RMII interface needs always a rate of 50MHz */
> +	err = clk_set_rate(priv->refclk, 50000000);
> +	if (err)
> +		dev_err(dev, "failed to change reference clock rate (%d)\n", err);
> +	return 0;
> +
> +out_regulator_disable:
> +	if (priv->regulator)
> +		regulator_disable(priv->regulator);
> +out_clk_disable:
> +	clk_disable_unprepare(priv->refclk);
> +out_netdev:
> +	free_netdev(ndev);
> +	return err;
> +}
> +
> +static int emac_rockchip_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct rockchip_priv_data *priv = netdev_priv(ndev);
> +	int err;
> +
> +	clk_disable_unprepare(priv->refclk);
> +
> +	if (priv->regulator)
> +		regulator_disable(priv->regulator);
> +
> +	err = arc_emac_remove(ndev);
> +	free_netdev(ndev);
> +	return err;
> +}
> +
> +static struct platform_driver emac_rockchip_driver = {
> +	.probe = emac_rockchip_probe,
> +	.remove = emac_rockchip_remove,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.of_match_table  = emac_rockchip_dt_ids,
> +	},
> +};
> +
> +module_platform_driver(emac_rockchip_driver);
> +
> +MODULE_AUTHOR("Romain Perier <romain.perier@gmail.com>");
> +MODULE_DESCRIPTION("Rockchip EMAC platform driver");
> +MODULE_LICENSE("GPL");

^ permalink raw reply

* Re: [patch net-next 01/13] openvswitch: split flow structures into ovs specific and generic ones
From: Jiri Pirko @ 2014-09-04 12:09 UTC (permalink / raw)
  To: John Fastabend
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
	buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
	vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <540731B9.4010603-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Wed, Sep 03, 2014 at 05:20:25PM CEST, john.fastabend-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>On 09/03/2014 02:24 AM, Jiri Pirko wrote:
>>After this, flow related structures can be used in other code.
>>
>>Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>>---
>
>Hi Jiri,
>
>As I indicated before I'm looking into integrating this with some
>hardware here. Progress is a bit slow but starting to look at it.The
>i40e/ixgbe driver being one open source example with very limited
>support for tables, flow matches, etc. And then a closed source driver
>with much more flexibility. What I don't have is a middle of the road
>switch to work with something better then a host nic but not as
>flexible as a TOR.
>
>Couple questions my assumption here is I can extend the flow_key
>as needed to support additional match criteria my hardware has.
>I scanned the ./net/openvswitch source and I didn't catch any
>place that would break but might need to take a closer look.
>Similarly the actions set will need to be extended. For example
>if I want to use this with i40e a OVS_ACTION_ATTR_QUEUE could
>be used to steer packets to the queue. With this in mind we
>will want a follow up patch to rename OVS_ACTION_ATTR_* to
>FLOW_ACTION_ATTR_*
>
>Also I have some filters that can match on offset/length/mask
>tuples. As far as I can tell this is going to have to be yet
>another interface? Or would it be worth the effort to define
>the flow key more generically. My initial guess is I'll just
>write a separate interface. I think this is what Jamal referred
>to as another "classifier".

I'm thinking about using some more generic match key. It would
incorporate ovs key and possible other classifiers (as your
off/len/mask) as well. Drivers will have free will to implement whatever
the hw supports.

Will do it for the next version of the patchset (most probably after I
return from holliday, Sep 15).


>
>Thanks,
>John
>
>[...]
>
>>+
>>+struct sw_flow_key_ipv4_tunnel {
>>+	__be64 tun_id;
>>+	__be32 ipv4_src;
>>+	__be32 ipv4_dst;
>>+	__be16 tun_flags;
>>+	u8   ipv4_tos;
>>+	u8   ipv4_ttl;
>>+};
>>+
>>+struct sw_flow_key {
>>+	struct sw_flow_key_ipv4_tunnel tun_key;  /* Encapsulating tunnel key. */
>>+	struct {
>>+		u32	priority;	/* Packet QoS priority. */
>>+		u32	skb_mark;	/* SKB mark. */
>>+		u16	in_port;	/* Input switch port (or DP_MAX_PORTS). */
>>+	} __packed phy; /* Safe when right after 'tun_key'. */
>>+	struct {
>>+		u8     src[ETH_ALEN];	/* Ethernet source address. */
>>+		u8     dst[ETH_ALEN];	/* Ethernet destination address. */
>>+		__be16 tci;		/* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */
>>+		__be16 type;		/* Ethernet frame type. */
>>+	} eth;
>>+	struct {
>>+		u8     proto;		/* IP protocol or lower 8 bits of ARP opcode. */
>>+		u8     tos;		/* IP ToS. */
>>+		u8     ttl;		/* IP TTL/hop limit. */
>>+		u8     frag;		/* One of OVS_FRAG_TYPE_*. */
>>+	} ip;
>>+	struct {
>>+		__be16 src;		/* TCP/UDP/SCTP source port. */
>>+		__be16 dst;		/* TCP/UDP/SCTP destination port. */
>>+		__be16 flags;		/* TCP flags. */
>>+	} tp;
>>+	union {
>>+		struct {
>>+			struct {
>>+				__be32 src;	/* IP source address. */
>>+				__be32 dst;	/* IP destination address. */
>>+			} addr;
>>+			struct {
>>+				u8 sha[ETH_ALEN];	/* ARP source hardware address. */
>>+				u8 tha[ETH_ALEN];	/* ARP target hardware address. */
>>+			} arp;
>>+		} ipv4;
>>+		struct {
>>+			struct {
>>+				struct in6_addr src;	/* IPv6 source address. */
>>+				struct in6_addr dst;	/* IPv6 destination address. */
>>+			} addr;
>>+			__be32 label;			/* IPv6 flow label. */
>>+			struct {
>>+				struct in6_addr target;	/* ND target address. */
>>+				u8 sll[ETH_ALEN];	/* ND source link layer address. */
>>+				u8 tll[ETH_ALEN];	/* ND target link layer address. */
>>+			} nd;
>>+		} ipv6;
>>+	};
>>+} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */
>>+
>>+struct sw_flow_key_range {
>>+	unsigned short int start;
>>+	unsigned short int end;
>>+};
>>+
>>+struct sw_flow_mask {
>>+	struct sw_flow_key_range range;
>>+	struct sw_flow_key key;
>>+};
>>+
>>+struct sw_flow_action {
>>+};
>>+
>>+struct sw_flow_actions {
>>+	unsigned count;
>>+	struct sw_flow_action actions[0];
>>+};
>>+
>>+struct sw_flow {
>>+	struct sw_flow_key key;
>>+	struct sw_flow_key unmasked_key;
>>+	struct sw_flow_mask *mask;
>>+	struct sw_flow_actions *actions;
>>+};
>>+
>
>
>-- 
>John Fastabend         Intel Corporation

^ permalink raw reply

* Re: [PATCH 3/4] ARM: dts: Add emac nodes to the rk3188 device tree
From: Heiko Stübner @ 2014-09-04 12:11 UTC (permalink / raw)
  To: Romain Perier; +Cc: linux-rockchip, linux-arm-kernel, netdev, devicetree, arnd
In-Reply-To: <1409763165-16461-3-git-send-email-romain.perier@gmail.com>

Am Mittwoch, 3. September 2014, 16:52:44 schrieb Romain Perier:
> This adds support for EMAC Rockchip driver on RK3188 SoCs.
> 
> Signed-off-by: Romain Perier <romain.perier@gmail.com>

could you think about changing patches 3 and 4 with the diff below?

For one the comment style is not really following the coding style.
I guess either remove the comments completely or switch to a /* ... */
form.

More importantly, I think the compatible is a property of the SoC and does
not need to be replicated into every board, so it should live in a small
&emac node in the rk3188.dtsi to adapt the property.


Heiko


diff --git a/arch/arm/boot/dts/rk3188-radxarock.dts b/arch/arm/boot/dts/rk3188-radxarock.dts
index 9f37c6f..a5c90e1 100644
--- a/arch/arm/boot/dts/rk3188-radxarock.dts
+++ b/arch/arm/boot/dts/rk3188-radxarock.dts
@@ -100,7 +100,6 @@
 
 &emac {
 	status = "okay";
-	compatible = "rockchip,rk3188-emac";
 
 	pinctrl-names = "default";
 	pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&phy_int>;
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 1261358..fd7511e 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -193,14 +193,14 @@
 
 		emac {
 			emac_xfer: emac-xfer {
-				rockchip,pins = <RK_GPIO3 16 RK_FUNC_2 &pcfg_pull_none>, //tx_en
-						<RK_GPIO3 17 RK_FUNC_2 &pcfg_pull_none>, //txd1
-						<RK_GPIO3 18 RK_FUNC_2 &pcfg_pull_none>, //txd0
-						<RK_GPIO3 19 RK_FUNC_2 &pcfg_pull_none>, //rxd0
-						<RK_GPIO3 20 RK_FUNC_2 &pcfg_pull_none>, //rxd1
-						<RK_GPIO3 21 RK_FUNC_2 &pcfg_pull_none>, //mac_clk
-						<RK_GPIO3 22 RK_FUNC_2 &pcfg_pull_none>, //rx_err
-						<RK_GPIO3 23 RK_FUNC_2 &pcfg_pull_none>; //crs_dvalid
+				rockchip,pins = <RK_GPIO3 16 RK_FUNC_2 &pcfg_pull_none>, /* tx_en */
+						<RK_GPIO3 17 RK_FUNC_2 &pcfg_pull_none>, /* txd1 */
+						<RK_GPIO3 18 RK_FUNC_2 &pcfg_pull_none>, /* txd0 */
+						<RK_GPIO3 19 RK_FUNC_2 &pcfg_pull_none>, /* rxd0 */
+						<RK_GPIO3 20 RK_FUNC_2 &pcfg_pull_none>, /* rxd1 */
+						<RK_GPIO3 21 RK_FUNC_2 &pcfg_pull_none>, /* mac_clk */
+						<RK_GPIO3 22 RK_FUNC_2 &pcfg_pull_none>, /* rx_err */
+						<RK_GPIO3 23 RK_FUNC_2 &pcfg_pull_none>; /* crs_dvalid */
 			};
 
 			emac_mdio: emac-mdio {
@@ -385,6 +385,10 @@
 	};
 };
 
+&emac {
+	compatible = "rockchip,rk3188-emac";
+};
+
 &global_timer {
 	interrupts = <GIC_PPI 11 0xf04>;
 };


> ---
>  arch/arm/boot/dts/rk3188.dtsi | 18 ++++++++++++++++++
>  arch/arm/boot/dts/rk3xxx.dtsi | 17 +++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
> index ee801a9..a182713 100644
> --- a/arch/arm/boot/dts/rk3188.dtsi
> +++ b/arch/arm/boot/dts/rk3188.dtsi
> @@ -147,6 +147,24 @@
>  			bias-disable;
>  		};
> 
> +		emac {
> +			emac_xfer: emac-xfer {
> +				rockchip,pins = <RK_GPIO3 16 RK_FUNC_2 &pcfg_pull_none>, //tx_en
> +						<RK_GPIO3 17 RK_FUNC_2 &pcfg_pull_none>, //txd1
> +						<RK_GPIO3 18 RK_FUNC_2 &pcfg_pull_none>, //txd0
> +						<RK_GPIO3 19 RK_FUNC_2 &pcfg_pull_none>, //rxd0
> +						<RK_GPIO3 20 RK_FUNC_2 &pcfg_pull_none>, //rxd1
> +						<RK_GPIO3 21 RK_FUNC_2 &pcfg_pull_none>, //mac_clk
> +						<RK_GPIO3 22 RK_FUNC_2 &pcfg_pull_none>, //rx_err
> +						<RK_GPIO3 23 RK_FUNC_2 &pcfg_pull_none>; //crs_dvalid
> +			};
> +
> +			emac_mdio: emac-mdio {
> +				rockchip,pins = <RK_GPIO3 24 RK_FUNC_2 &pcfg_pull_none>,
> +						<RK_GPIO3 25 RK_FUNC_2 &pcfg_pull_none>;
> +			};
> +		};
> +
>  		i2c0 {
>  			i2c0_xfer: i2c0-xfer {
>  				rockchip,pins = <RK_GPIO1 24 RK_FUNC_1 &pcfg_pull_none>,
> diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
> index 8caf85d..208b1df 100644
> --- a/arch/arm/boot/dts/rk3xxx.dtsi
> +++ b/arch/arm/boot/dts/rk3xxx.dtsi
> @@ -91,6 +91,23 @@
>  		status = "disabled";
>  	};
> 
> +	emac: ethernet@10204000 {
> +		compatible = "snps,arc-emac";
> +		reg = <0x10204000 0x3c>;
> +		interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		rockchip,grf = <&grf>;
> +
> +		clocks = <&cru HCLK_EMAC>, <&cru SCLK_MAC>;
> +		clock-names = "hclk", "macref";
> +		max-speed = <100>;
> +		phy-mode = "rmii";
> +
> +		status = "disabled";
> +	};
> +
>  	mmc0: dwmmc@10214000 {
>  		compatible = "rockchip,rk2928-dw-mshc";
>  		reg = <0x10214000 0x1000>;

^ permalink raw reply related

* Re: [PATCH v2 2/4] dt-bindings: Document EMAC Rockchip
From: Heiko Stübner @ 2014-09-04 12:13 UTC (permalink / raw)
  To: Romain Perier; +Cc: linux-rockchip, linux-arm-kernel, netdev, devicetree, arnd
In-Reply-To: <1409763165-16461-2-git-send-email-romain.perier@gmail.com>

Am Mittwoch, 3. September 2014, 16:52:43 schrieb Romain Perier:
> This adds the necessary binding documentation for the EMAC Rockchip platform
> driver found in RK3066 and RK3188 SoCs.
> 
> Signed-off-by: Romain Perier <romain.perier@gmail.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  .../devicetree/bindings/net/emac_rockchip.txt      | 50
> ++++++++++++++++++++++ 1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/emac_rockchip.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/emac_rockchip.txt
> b/Documentation/devicetree/bindings/net/emac_rockchip.txt new file mode
> 100644
> index 0000000..8dc1c79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/emac_rockchip.txt
> @@ -0,0 +1,50 @@
> +* ARC EMAC 10/100 Ethernet platform driver for Rockchip Rk3066/RK3188 SoCs
> +
> +Required properties:
> +- compatible: Should be "rockchip,rk3066-emac" or "rockchip,rk3188-emac"
> +  according to the target SoC.
> +- reg: Address and length of the register set for the device
> +- interrupts: Should contain the EMAC interrupts
> +- rockchip,grf: phandle to the syscon grf used to control speed and mode
> +  for emac.
> +- phy: see ethernet.txt file in the same directory.
> +- phy-mode: see ethernet.txt file in the same directory.
> +
> +Optional properties:
> +- phy-supply: phandle to a regulator if the PHY needs one
> +
> +Clock handling:
> +- clocks: Must contain an entry for each entry in clock-names.
> +- clock-names: Shall be "hclk" for the host clock needed to calculate and
> set +  polling period of EMAC and "macref" for the reference clock needed
> to transfer +  data to and from the phy.
> +
> +Child nodes of the driver are the individual PHY devices connected to the
> +MDIO bus. They must have a "reg" property given the PHY address on the MDIO
> bus. +
> +Examples:
> +
> +ethernet@10204000 {
> +	compatible = "rockchip,rk3188-emac";
> +	reg = <0xc0fc2000 0x3c>;
> +	interrupts = <6>;
> +	mac-address = [ 00 11 22 33 44 55 ];
> +
> +	clocks = <&cru HCLK_EMAC>, <&cru SCLK_MAC>;
> +	clock-names = "hclk", "macref";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&phy_int>;
> +
> +	rockchip,grf = <&grf>;
> +
> +	phy = <&phy0>;
> +	phy-mode = "rmii";
> +	phy-supply = <&vcc_rmii>;
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	phy0: ethernet-phy@0 {
> +	      reg = <1>;
> +	};
> +};

^ permalink raw reply

* Re: [patch net-next 01/13] openvswitch: split flow structures into ovs specific and generic ones
From: Jiri Pirko @ 2014-09-04 12:25 UTC (permalink / raw)
  To: Pravin Shelar
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, Rony Efraim,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ, Eric Dumazet,
	andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	nbd-p3rKhJxN3npAfugRpC6u6w, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	John Fastabend, jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	Or Gerlitz, Ben Hutchings, buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	Nicolas Dichtel, vyasevic-H+wXaHxf7aLQT0dZR+AlfA,
	nhorman-2XuSBdqkA4R54TAoqtyWWQ, netdev, Stephen Hemminger,
	Daniel Borkmann, ebiederm-aS9lmoZGLiVWk0Htik3J/w, David Miller
In-Reply-To: <CALnjE+rk26Om1O5_Q=8tn7eAyh4Ywen-1+UD_nCVj_geZY1HuQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Wed, Sep 03, 2014 at 08:42:18PM CEST, pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org wrote:
>On Wed, Sep 3, 2014 at 8:20 AM, John Fastabend <john.fastabend-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 09/03/2014 02:24 AM, Jiri Pirko wrote:
>>>
>>> After this, flow related structures can be used in other code.
>>>
>>> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>>> ---
>>
>>
>> Hi Jiri,
>>
>> As I indicated before I'm looking into integrating this with some
>> hardware here. Progress is a bit slow but starting to look at it.The
>> i40e/ixgbe driver being one open source example with very limited
>> support for tables, flow matches, etc. And then a closed source driver
>> with much more flexibility. What I don't have is a middle of the road
>> switch to work with something better then a host nic but not as
>> flexible as a TOR.
>>
>> Couple questions my assumption here is I can extend the flow_key
>> as needed to support additional match criteria my hardware has.
>> I scanned the ./net/openvswitch source and I didn't catch any
>> place that would break but might need to take a closer look.
>> Similarly the actions set will need to be extended. For example
>> if I want to use this with i40e a OVS_ACTION_ATTR_QUEUE could
>> be used to steer packets to the queue. With this in mind we
>> will want a follow up patch to rename OVS_ACTION_ATTR_* to
>> FLOW_ACTION_ATTR_*
>>
>
>struct sw_flow_key is internal structure of OVS, it is designed to
>have better flow-table performance. By adding hw specific fields in
>sw_flow_key, it increase flow-key size and that has negative impact on
>OVS software switching performance. Therefore it is better not to
>share this internal structure with driver interface.

Ok. I will split this leaving the sw_flow_key into ovs and introducing
new one. Thanks.

>
>Thanks.
>
>> Also I have some filters that can match on offset/length/mask
>> tuples. As far as I can tell this is going to have to be yet
>> another interface? Or would it be worth the effort to define
>> the flow key more generically. My initial guess is I'll just
>> write a separate interface. I think this is what Jamal referred
>> to as another "classifier".
>>
>> Thanks,
>> John
>>
>> [...]
>>
>>
>>> +
>>> +struct sw_flow_key_ipv4_tunnel {
>>> +       __be64 tun_id;
>>> +       __be32 ipv4_src;
>>> +       __be32 ipv4_dst;
>>> +       __be16 tun_flags;
>>> +       u8   ipv4_tos;
>>> +       u8   ipv4_ttl;
>>> +};
>>> +
>>> +struct sw_flow_key {
>>> +       struct sw_flow_key_ipv4_tunnel tun_key;  /* Encapsulating tunnel
>>> key. */
>>> +       struct {
>>> +               u32     priority;       /* Packet QoS priority. */
>>> +               u32     skb_mark;       /* SKB mark. */
>>> +               u16     in_port;        /* Input switch port (or
>>> DP_MAX_PORTS). */
>>> +       } __packed phy; /* Safe when right after 'tun_key'. */
>>> +       struct {
>>> +               u8     src[ETH_ALEN];   /* Ethernet source address. */
>>> +               u8     dst[ETH_ALEN];   /* Ethernet destination address.
>>> */
>>> +               __be16 tci;             /* 0 if no VLAN, VLAN_TAG_PRESENT
>>> set otherwise. */
>>> +               __be16 type;            /* Ethernet frame type. */
>>> +       } eth;
>>> +       struct {
>>> +               u8     proto;           /* IP protocol or lower 8 bits of
>>> ARP opcode. */
>>> +               u8     tos;             /* IP ToS. */
>>> +               u8     ttl;             /* IP TTL/hop limit. */
>>> +               u8     frag;            /* One of OVS_FRAG_TYPE_*. */
>>> +       } ip;
>>> +       struct {
>>> +               __be16 src;             /* TCP/UDP/SCTP source port. */
>>> +               __be16 dst;             /* TCP/UDP/SCTP destination port.
>>> */
>>> +               __be16 flags;           /* TCP flags. */
>>> +       } tp;
>>> +       union {
>>> +               struct {
>>> +                       struct {
>>> +                               __be32 src;     /* IP source address. */
>>> +                               __be32 dst;     /* IP destination address.
>>> */
>>> +                       } addr;
>>> +                       struct {
>>> +                               u8 sha[ETH_ALEN];       /* ARP source
>>> hardware address. */
>>> +                               u8 tha[ETH_ALEN];       /* ARP target
>>> hardware address. */
>>> +                       } arp;
>>> +               } ipv4;
>>> +               struct {
>>> +                       struct {
>>> +                               struct in6_addr src;    /* IPv6 source
>>> address. */
>>> +                               struct in6_addr dst;    /* IPv6
>>> destination address. */
>>> +                       } addr;
>>> +                       __be32 label;                   /* IPv6 flow
>>> label. */
>>> +                       struct {
>>> +                               struct in6_addr target; /* ND target
>>> address. */
>>> +                               u8 sll[ETH_ALEN];       /* ND source link
>>> layer address. */
>>> +                               u8 tll[ETH_ALEN];       /* ND target link
>>> layer address. */
>>> +                       } nd;
>>> +               } ipv6;
>>> +       };
>>> +} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as
>>> longs. */
>>> +
>>> +struct sw_flow_key_range {
>>> +       unsigned short int start;
>>> +       unsigned short int end;
>>> +};
>>> +
>>> +struct sw_flow_mask {
>>> +       struct sw_flow_key_range range;
>>> +       struct sw_flow_key key;
>>> +};
>>> +
>>> +struct sw_flow_action {
>>> +};
>>> +
>>> +struct sw_flow_actions {
>>> +       unsigned count;
>>> +       struct sw_flow_action actions[0];
>>> +};
>>> +
>>> +struct sw_flow {
>>> +       struct sw_flow_key key;
>>> +       struct sw_flow_key unmasked_key;
>>> +       struct sw_flow_mask *mask;
>>> +       struct sw_flow_actions *actions;
>>> +};
>>> +
>>
>>
>>
>> --
>> John Fastabend         Intel Corporation

^ permalink raw reply

* Re: [patch net-next 01/13] openvswitch: split flow structures into ovs specific and generic ones
From: Jiri Pirko @ 2014-09-04 12:33 UTC (permalink / raw)
  To: Pravin Shelar
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ, Eric Dumazet,
	andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	nbd-p3rKhJxN3npAfugRpC6u6w, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	Rony Efraim, jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w, Or Gerlitz,
	Ben Hutchings, buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	Nicolas Dichtel, vyasevic-H+wXaHxf7aLQT0dZR+AlfA,
	nhorman-2XuSBdqkA4R54TAoqtyWWQ, netdev, Stephen Hemminger,
	Daniel Borkmann, ebiederm-aS9lmoZGLiVWk0Htik3J/w, David Miller
In-Reply-To: <CALnjE+pscRmfhaWgkWCunJfjvG04RiNUAj6nefSFHrknQTC+xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Wed, Sep 03, 2014 at 08:41:39PM CEST, pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org wrote:
>On Wed, Sep 3, 2014 at 2:24 AM, Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org> wrote:
>> After this, flow related structures can be used in other code.
>>
>> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>> ---
>>  include/net/sw_flow.h          |  99 ++++++++++++++++++++++++++++++++++
>>  net/openvswitch/actions.c      |   3 +-
>>  net/openvswitch/datapath.c     |  74 +++++++++++++-------------
>>  net/openvswitch/datapath.h     |   4 +-
>>  net/openvswitch/flow.c         |   6 +--
>>  net/openvswitch/flow.h         | 102 +++++++----------------------------
>>  net/openvswitch/flow_netlink.c |  53 +++++++++---------
>>  net/openvswitch/flow_netlink.h |  10 ++--
>>  net/openvswitch/flow_table.c   | 118 ++++++++++++++++++++++-------------------
>>  net/openvswitch/flow_table.h   |  30 +++++------
>>  net/openvswitch/vport-gre.c    |   4 +-
>>  net/openvswitch/vport-vxlan.c  |   2 +-
>>  net/openvswitch/vport.c        |   2 +-
>>  net/openvswitch/vport.h        |   2 +-
>>  14 files changed, 276 insertions(+), 233 deletions(-)
>>  create mode 100644 include/net/sw_flow.h
>>
>> diff --git a/include/net/sw_flow.h b/include/net/sw_flow.h
>> new file mode 100644
>> index 0000000..21724f1
>> --- /dev/null
>> +++ b/include/net/sw_flow.h
>> @@ -0,0 +1,99 @@
>> +/*
>> + * include/net/sw_flow.h - Generic switch flow structures
>> + * Copyright (c) 2007-2012 Nicira, Inc.
>> + * Copyright (c) 2014 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#ifndef _NET_SW_FLOW_H_
>> +#define _NET_SW_FLOW_H_
>> +
>> +struct sw_flow_key_ipv4_tunnel {
>> +       __be64 tun_id;
>> +       __be32 ipv4_src;
>> +       __be32 ipv4_dst;
>> +       __be16 tun_flags;
>> +       u8   ipv4_tos;
>> +       u8   ipv4_ttl;
>> +};
>> +
>> +struct sw_flow_key {
>> +       struct sw_flow_key_ipv4_tunnel tun_key;  /* Encapsulating tunnel key. */
>> +       struct {
>> +               u32     priority;       /* Packet QoS priority. */
>> +               u32     skb_mark;       /* SKB mark. */
>> +               u16     in_port;        /* Input switch port (or DP_MAX_PORTS). */
>> +       } __packed phy; /* Safe when right after 'tun_key'. */
>> +       struct {
>> +               u8     src[ETH_ALEN];   /* Ethernet source address. */
>> +               u8     dst[ETH_ALEN];   /* Ethernet destination address. */
>> +               __be16 tci;             /* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */
>> +               __be16 type;            /* Ethernet frame type. */
>> +       } eth;
>> +       struct {
>> +               u8     proto;           /* IP protocol or lower 8 bits of ARP opcode. */
>> +               u8     tos;             /* IP ToS. */
>> +               u8     ttl;             /* IP TTL/hop limit. */
>> +               u8     frag;            /* One of OVS_FRAG_TYPE_*. */
>> +       } ip;
>> +       struct {
>> +               __be16 src;             /* TCP/UDP/SCTP source port. */
>> +               __be16 dst;             /* TCP/UDP/SCTP destination port. */
>> +               __be16 flags;           /* TCP flags. */
>> +       } tp;
>> +       union {
>> +               struct {
>> +                       struct {
>> +                               __be32 src;     /* IP source address. */
>> +                               __be32 dst;     /* IP destination address. */
>> +                       } addr;
>> +                       struct {
>> +                               u8 sha[ETH_ALEN];       /* ARP source hardware address. */
>> +                               u8 tha[ETH_ALEN];       /* ARP target hardware address. */
>> +                       } arp;
>> +               } ipv4;
>> +               struct {
>> +                       struct {
>> +                               struct in6_addr src;    /* IPv6 source address. */
>> +                               struct in6_addr dst;    /* IPv6 destination address. */
>> +                       } addr;
>> +                       __be32 label;                   /* IPv6 flow label. */
>> +                       struct {
>> +                               struct in6_addr target; /* ND target address. */
>> +                               u8 sll[ETH_ALEN];       /* ND source link layer address. */
>> +                               u8 tll[ETH_ALEN];       /* ND target link layer address. */
>> +                       } nd;
>> +               } ipv6;
>> +       };
>> +} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */
>> +
>
>HW offload API should be separate from OVS module. This has following
>advantages.
>1. It can be managed by OVS userspace vswitchd process which has much
>better context to setup hardware flow table. Once we add capabilities
>for swdev, it is much more easier for vswitchd process to choose
>correct (hw or sw) flow table for given flow.

The idea is to add a nl attr in ovs genl iface so the vswitchd can
speficify the flow the to be in sw only, in hw only, in both.
I believe that is is more convenient to let switchd to communicate flows
via single iface.

>2. Other application that wants to use HW offload does not have
>dependency on OVS kernel module.

That is not the case for this patchset. Userspace can insert/remove
flows using the switchdev generic netlink api - see:
[patch net-next 13/13] switchdev: introduce Netlink API

>3. Hardware and software datapath remains separate, these two
>components has no dependency on each other, both can be developed
>independent of each other.


The general idea is to have the offloads handled in-kernel. Therefore I
hooked on to ovs kernel dp code.

^ permalink raw reply

* Re: [patch net-next 03/13] net: introduce generic switch devices support
From: Jiri Pirko @ 2014-09-04 12:46 UTC (permalink / raw)
  To: John Fastabend
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
	buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
	vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <540737CF.4000402-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Wed, Sep 03, 2014 at 05:46:23PM CEST, john.fastabend-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>On 09/03/2014 02:24 AM, Jiri Pirko wrote:
>>The goal of this is to provide a possibility to suport various switch
>>chips. Drivers should implement relevant ndos to do so. Now there is a
>>couple of ndos defines:
>>- for getting physical switch id is in place.
>>- for work with flows.
>>
>>Note that user can use random port netdevice to access the switch.
>>
>>Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>>---
>
>
>[...]
>
>>  struct netpoll_info;
>>@@ -997,6 +999,24 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>   *	Callback to use for xmit over the accelerated station. This
>>   *	is used in place of ndo_start_xmit on accelerated net
>>   *	devices.
>>+ *
>>+ * int (*ndo_swdev_get_id)(struct net_device *dev,
>>+ *			   struct netdev_phys_item_id *psid);
>>+ *	Called to get an ID of the switch chip this port is part of.
>>+ *	If driver implements this, it indicates that it represents a port
>>+ *	of a switch chip.
>>+ *
>>+ * int (*ndo_swdev_flow_insert)(struct net_device *dev,
>>+ *				const struct sw_flow *flow);
>>+ *	Called to insert a flow into switch device. If driver does
>>+ *	not implement this, it is assumed that the hw does not have
>>+ *	a capability to work with flows.
>>+ *
>>+ * int (*ndo_swdev_flow_remove)(struct net_device *dev,
>>+ *				const struct sw_flow *flow);
>>+ *	Called to remove a flow from switch device. If driver does
>>+ *	not implement this, it is assumed that the hw does not have
>>+ *	a capability to work with flows.
>>   */
>>  struct net_device_ops {
>>  	int			(*ndo_init)(struct net_device *dev);
>>@@ -1146,6 +1166,14 @@ struct net_device_ops {
>>  							struct net_device *dev,
>>  							void *priv);
>>  	int			(*ndo_get_lock_subclass)(struct net_device *dev);
>>+#ifdef CONFIG_NET_SWITCHDEV
>>+	int			(*ndo_swdev_get_id)(struct net_device *dev,
>>+						    struct netdev_phys_item_id *psid);
>>+	int			(*ndo_swdev_flow_insert)(struct net_device *dev,
>>+							 const struct sw_flow *flow);
>>+	int			(*ndo_swdev_flow_remove)(struct net_device *dev,
>>+							 const struct sw_flow *flow);
>
>Not really a critique of your patch but I'll need to extend this
>with a ndo_swdev_flow_dump() to get the fields. Without this if
>your user space side ever restarts, gets out of sync there is no
>way to get back in sync.

Sure. I do not say that the api is complete (If anything ever is...)
Feel free to add dump ndo. In fact we can take care of it and implement
in rocker driver.


>
>Also with hardware that has multiple flow tables we need to indicate
>the table to insert the flow into. One concrete reason to do this
>is to create atomic updates of multiple ACLs. The idea is to create
>a new ACL table build the table up and then link it in. This can be
>added when its needed my opensource drivers don't support this yet
>either but maybe adding multiple tables to rocker switch will help
>flush this out.

Ok. Lets leave this for future follow-ups.

>
>Finally we need some way to drive capabilities out of the swdev.
>Even rocker switch needs this to indicate it doesn't support matching
>on all the sw_flow fields. Without this its not clear to me how to
>manage the device from user space. I tried writing user space daemon
>for the simpler flow director interface and the try and see model
>breaks quickly.

Hmm. I was under impression that a simple fact that the flow insertion
fails with error is enough. But thining of it more. I believe that a set
of features makes sense. I will think about it and add it into the next
patchset version.

>
>>+#endif
>>  };
>>
>>  /**
>>diff --git a/include/net/sw_flow.h b/include/net/sw_flow.h
>>index 21724f1..3af7758 100644
>>--- a/include/net/sw_flow.h
>>+++ b/include/net/sw_flow.h
>>@@ -81,7 +81,21 @@ struct sw_flow_mask {
>>  	struct sw_flow_key key;
>>  };
>>
>>+enum sw_flow_action_type {
>>+	SW_FLOW_ACTION_TYPE_OUTPUT,
>>+	SW_FLOW_ACTION_TYPE_VLAN_PUSH,
>>+	SW_FLOW_ACTION_TYPE_VLAN_POP,
>>+};
>>+
>
>OK my previous comment about having another patch to create
>generic actions seems to be resolved here. I'm not sure how
>important it is but if we abstract the flow types away from
>OVS is there any reason not to reuse and relabel the action
>types as well? I guess we can't break userspace API but maybe
>a 1:1 mapping would be better?
>
>>  struct sw_flow_action {
>>+	enum sw_flow_action_type type;
>>+	union {
>>+		u32 out_port_ifindex;
>>+		struct {
>>+			__be16 vlan_proto;
>>+			u16 vlan_tci;
>>+		} vlan;
>>+	};
>>  };
>
>[...]
>
>I think my comments could be addressed with additional patches
>if you want. I could help but it will be another week or so
>before I have some time. The biggest issue IMO is the lack of
>capabilities queries.

Np. I will handle these (probably not before I return from vacation (Sep
15)).

Thanks!

^ permalink raw reply

* Re: [patch net-next 07/13] dsa: implement ndo_swdev_get_id
From: Jiri Pirko @ 2014-09-04 12:47 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
	ronye-VPRAkNaXOzVWk0Htik3J/w,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
	buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
	vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <5407A25A.8050401-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thu, Sep 04, 2014 at 01:20:58AM CEST, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>On 09/03/2014 02:24 AM, Jiri Pirko wrote:
>> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>> ---
>>  include/linux/netdevice.h |  3 ++-
>>  include/net/dsa.h         |  1 +
>>  net/dsa/Kconfig           |  2 +-
>>  net/dsa/dsa.c             |  3 +++
>>  net/dsa/slave.c           | 10 ++++++++++
>>  5 files changed, 17 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 6a009d1..7ee070f 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -41,7 +41,6 @@
>>  
>>  #include <linux/ethtool.h>
>>  #include <net/net_namespace.h>
>> -#include <net/dsa.h>
>>  #ifdef CONFIG_DCB
>>  #include <net/dcbnl.h>
>>  #endif
>> @@ -1259,6 +1258,8 @@ enum netdev_priv_flags {
>>  #define IFF_LIVE_ADDR_CHANGE		IFF_LIVE_ADDR_CHANGE
>>  #define IFF_MACVLAN			IFF_MACVLAN
>>  
>> +#include <net/dsa.h>
>> +
>>  /**
>>   *	struct net_device - The DEVICE structure.
>>   *		Actually, this whole structure is a big mistake.  It mixes I/O
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index 9771292..d60cd42 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -140,6 +140,7 @@ struct dsa_switch {
>>  	u32			phys_mii_mask;
>>  	struct mii_bus		*slave_mii_bus;
>>  	struct net_device	*ports[DSA_MAX_PORTS];
>> +	struct netdev_phys_item_id psid;
>>  };
>>  
>>  static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p)
>> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
>> index a585fd6..4e144a2 100644
>> --- a/net/dsa/Kconfig
>> +++ b/net/dsa/Kconfig
>> @@ -1,6 +1,6 @@
>>  config HAVE_NET_DSA
>>  	def_bool y
>> -	depends on NETDEVICES && !S390
>> +	depends on NETDEVICES && NET_SWITCHDEV && !S390
>
>It does not look like this is necessary, we are only using definitions
>from net/dsa.h and include/linux/netdevice.h, and if it was, a 'select'
>would be more appropriate here I think.
>
>TBH, I think we should rather drop this patch for now, I do not see any
>benefit in providing a random id over no-id at all.

Well, the benefit is that you are still able to see which ports belong
to the same switch.

^ permalink raw reply

* Re: [patch net-next 10/13] openvswitch: add support for datapath hardware offload
From: Jiri Pirko @ 2014-09-04 12:48 UTC (permalink / raw)
  To: John Fastabend
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
	buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
	vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <540743B4.9080500-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Wed, Sep 03, 2014 at 06:37:08PM CEST, john.fastabend-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>On 09/03/2014 02:24 AM, Jiri Pirko wrote:
>>Benefit from the possibility to work with flows in switch devices and
>>use the swdev api to offload flow datapath.
>>
>>Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>>---
>>  net/openvswitch/Makefile       |   3 +-
>>  net/openvswitch/datapath.c     |  33 ++++++
>>  net/openvswitch/datapath.h     |   3 +
>>  net/openvswitch/flow_table.c   |   1 +
>>  net/openvswitch/hw_offload.c   | 245 +++++++++++++++++++++++++++++++++++++++++
>>  net/openvswitch/hw_offload.h   |  22 ++++
>>  net/openvswitch/vport-netdev.c |   3 +
>>  net/openvswitch/vport.h        |   2 +
>>  8 files changed, 311 insertions(+), 1 deletion(-)
>>  create mode 100644 net/openvswitch/hw_offload.c
>>  create mode 100644 net/openvswitch/hw_offload.h
>>
>>diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile
>>index 3591cb5..5152437 100644
>>--- a/net/openvswitch/Makefile
>>+++ b/net/openvswitch/Makefile
>>@@ -13,7 +13,8 @@ openvswitch-y := \
>>  	flow_table.o \
>>  	vport.o \
>>  	vport-internal_dev.o \
>>-	vport-netdev.o
>>+	vport-netdev.o \
>>+	hw_offload.o
>>
>>  ifneq ($(CONFIG_OPENVSWITCH_VXLAN),)
>>  openvswitch-y += vport-vxlan.o
>>diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
>>index 75bb07f..3e43e1d 100644
>>--- a/net/openvswitch/datapath.c
>>+++ b/net/openvswitch/datapath.c
>>@@ -57,6 +57,7 @@
>>  #include "flow_netlink.h"
>>  #include "vport-internal_dev.h"
>>  #include "vport-netdev.h"
>>+#include "hw_offload.h"
>>
>>  int ovs_net_id __read_mostly;
>>
>>@@ -864,6 +865,9 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
>>  			acts = NULL;
>>  			goto err_unlock_ovs;
>>  		}
>>+		error = ovs_hw_flow_insert(dp, new_flow);
>>+		if (error)
>>+			pr_warn("failed to insert flow into hw\n");
>
>This is really close to silently failing. I think we need to
>hard fail here somehow and push it back to userspace as part of
>the reply and ovs_notify.

Yes, I agree. My plan was to handle this in ovs hw/sw/both netlink attr
implementation.


>
>Otherwise I don't know how to manage the hardware correctly. Consider
>the hardware table is full. In this case user space will continue to
>add rules and they will be silently discarded. Similarly if user space
>adds a flow/action that can not be supported by the hardware it will
>be silently ignored.
>
>Even if we do careful accounting on resources in user space we could
>still get an ENOMEM error from sw_flow_action_create.
>
>Same comment for the other hw commands flush/remove.
>
>>  		if (unlikely(reply)) {
>>  			error = ovs_flow_cmd_fill_info(new_flow,
>>@@ -896,10 +900,18 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
>>  				goto err_unlock_ovs;
>>  			}
>>  		}
>
>
>[...]
>
>
>Thanks,
>John
>
>-- 
>John Fastabend         Intel Corporation

^ permalink raw reply

* [RFC net-next PATCH V2 0/3] qdisc bulk dequeuing and utilizing delayed tailptr updates
From: Jesper Dangaard Brouer @ 2014-09-04 12:54 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
	Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
	Daniel Borkmann
  Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend

Wanted people to review this work early... so this is the current
state, even added my debug patch, if people want to "see" it work.

I'm currently testing with different combination of netperf, and
different on/off adjustments of {gso,gro,tso} as this influences the
code path / skb list generation vs. real GSOs.

Open questions:

- For now set bulk limit to 6 packets (counting including the head),
  this need some user adjustable param.  I'm open to suggestions?
  Perhaps we should start with 2 packets, or disable it as default?

- We are not doing proper accounting for weight_p/quota in
  __qdisc_run().  Should we reduce change this.  Open to suggestions.

---

Jesper Dangaard Brouer (2):
      qdisc: debug statements while testing prev-patch
      qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE

Tom Herbert (1):
      net: Functions to report space available in device TX queues


 include/linux/netdevice.h |   28 ++++++++++++-
 net/sched/sch_generic.c   |   95 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 118 insertions(+), 5 deletions(-)

^ permalink raw reply

* [RFC net-next PATCH V2 1/3] net: Functions to report space available in device TX queues
From: Jesper Dangaard Brouer @ 2014-09-04 12:54 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
	Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
	Daniel Borkmann
  Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140904125247.4108.8132.stgit@dragon>

From: Tom Herbert <therbert@google.com>

This patch adds netdev_tx_avail_queue and netdev_avail_queue which are
used to report number of bytes available in transmit queues per BQL. The
functions call dql_avail which returns BQL limit minus number of
inflight bytes. These functions can be called without txlock, for
instance to ascertain how much data should be dequeued from a qdisc in
a batch. When called without the tx_lock, the result is technically a
hint, subsequently when the tx_lock is done for a transmit it is
possible the availability has changed (for example a transmit
completion may have freed up more space in the queue or changed the
limit).

Signed-off-by: Tom Herbert <therbert@google.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

---
 - Fixed spelling in comments

 include/linux/netdevice.h |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5be20a7..5a4384a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2567,6 +2567,30 @@ static inline void netdev_completed_queue(struct net_device *dev,
 	netdev_tx_completed_queue(netdev_get_tx_queue(dev, 0), pkts, bytes);
 }
 
+static inline int netdev_tx_avail_queue(struct netdev_queue *dev_queue)
+{
+#ifdef CONFIG_BQL
+	return dql_avail(&dev_queue->dql);
+#else
+	return DQL_MAX_LIMIT;
+#endif
+}
+
+/**
+ *	netdev_avail_queue - report how much space is available for xmit
+ *	@dev: network device
+ *
+ *	Report the amount of space available in the TX queue in terms of
+ *	number of bytes. This returns the number of bytes available per
+ *	DQL. This function may be called without taking the txlock on
+ *	the device, however in that case the result should be taken as
+ *	a (strong) hint.
+ */
+static inline int netdev_avail_queue(struct net_device *dev)
+{
+	return netdev_tx_avail_queue(netdev_get_tx_queue(dev, 0));
+}
+
 static inline void netdev_tx_reset_queue(struct netdev_queue *q)
 {
 #ifdef CONFIG_BQL
@@ -2582,9 +2606,9 @@ static inline void netdev_tx_reset_queue(struct netdev_queue *q)
  * 	Reset the bytes and packet count of a network device and clear the
  * 	software flow control OFF bit for this network device
  */
-static inline void netdev_reset_queue(struct net_device *dev_queue)
+static inline void netdev_reset_queue(struct net_device *dev)
 {
-	netdev_tx_reset_queue(netdev_get_tx_queue(dev_queue, 0));
+	netdev_tx_reset_queue(netdev_get_tx_queue(dev, 0));
 }
 
 /**

^ permalink raw reply related

* Re: [Patch net-next 10/11] ARM: Documentation: Update fec dts binding doc
From: Sergei Shtylyov @ 2014-09-04 12:55 UTC (permalink / raw)
  To: y, b38611, davem, netdev, lznuaa
  Cc: shawn.guo, linux-arm-kernel, devicetree, Frank Li
In-Reply-To: <54078488.68c1420a.3020.7c0dSMTPIN_ADDED_BROKEN@mx.google.com>

Hello.

On 9/4/2014 12:34 AM, y@shlinux1.ap.freescale.net wrote:

> From: Frank Li <Frank.Li@freescale.com>

>      This patch update fec devicetree binding doc that add Optional
>      properties "fsl,num_tx_queues" and "fsl,num_rx_queues".

> Signed-off-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
>   Documentation/devicetree/bindings/net/fsl-fec.txt | 6 ++++++
>   1 file changed, 6 insertions(+)

> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index 8a2c7b5..4da148d 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -16,6 +16,12 @@ Optional properties:
>   - phy-handle : phandle to the PHY device connected to this device.
>   - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
>     Use instead of phy-handle.
> +- fsl,num_tx_queues : The property is valid for enet-avb IP, which supports
> +  hw multi queues. Should specify the tx queue number, otherwise set tx queue
> +  number to 1.
> +- fsl,num_rx_queues : The property is valid for enet-avb IP, which supports
> +  hw multi queues. Should specify the rx queue number, otherwise set rx queue
> +  number to 1.

    Hyphens are preferred to underscores in the prop names.

WBR, Sergei

^ permalink raw reply

* [RFC net-next PATCH V2 2/3] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Jesper Dangaard Brouer @ 2014-09-04 12:55 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
	Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
	Daniel Borkmann
  Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140904125247.4108.8132.stgit@dragon>

Based on DaveM's recent API work on dev_hard_start_xmit(), that allows
sending/processing an entire skb list.

This patch implements qdisc bulk dequeue, by allowing multiple packets
to be dequeued in dequeue_skb().

The optimization principle for this is two fold, (1) to amortize
locking cost and (2) avoid expensive tailptr update for notifying HW.
 (1) Several packets are dequeued while holding the qdisc root_lock,
amortizing locking cost over several packet.  The dequeued SKB list is
processed under the TXQ lock in dev_hard_start_xmit(), thus also
amortizing the cost of the TXQ lock.
 (2) Further more, dev_hard_start_xmit() will utilize the skb->xmit_more
API to delay HW tailptr update, which also reduces the cost per
packet.

One restriction of the new API is that every SKB must belong to the
same TXQ.  This patch takes the easy way out, by restricting bulk
dequeue to qdisc's with the TCQ_F_ONETXQUEUE flag, that specifies the
qdisc only have attached a single TXQ.

Some detail about the flow; dev_hard_start_xmit() will process the skb
list, and transmit packets individually towards the driver (see
xmit_one()).  In case the driver stops midway in the list, the
remaining skb list is returned by dev_hard_start_xmit().  In
sch_direct_xmit() this returned list is requeued by dev_requeue_skb().

The patch also tries to limit the amount of bytes dequeued, based on
the drivers BQL limits.  It also tries to avoid and stop dequeuing
when seeing a GSO packet (both real GSO and segmented GSO skb lists).

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

---
V2:
 - Restruct functions, split out functionality
 - Use BQL bytelimit to avoid overshooting driver limits, causing
   too large skb lists to be sitting on the requeue gso_skb.

 net/sched/sch_generic.c |   67 +++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 19696eb..a0c8070 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -56,6 +56,67 @@ static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
 	return 0;
 }
 
+static inline bool qdisc_may_bulk(const struct Qdisc *qdisc,
+				  const struct sk_buff *skb)
+{
+	return (qdisc->flags & TCQ_F_ONETXQUEUE);
+}
+
+static inline struct sk_buff *qdisc_dequeue_validate(struct Qdisc *qdisc)
+{
+	struct sk_buff *skb = qdisc->dequeue(qdisc);
+
+	if (skb != NULL)
+		skb = validate_xmit_skb(skb, qdisc_dev(qdisc));
+
+	return skb;
+}
+
+static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
+						     struct sk_buff *head)
+{
+	struct sk_buff *new, *skb = head;
+	struct netdev_queue *txq = q->dev_queue;
+	int bytelimit = netdev_tx_avail_queue(txq);
+	int limit = 5;
+
+	if (bytelimit <= 0)
+		return head;
+
+	do {
+		if (skb->next || skb_is_gso(skb)) {
+			/* Stop processing if the skb is already a skb
+			 * list (e.g a segmented GSO packet) or a real
+			 * GSO packet */
+			break;
+		}
+		new = qdisc_dequeue_validate(q);
+		if (new) {
+			skb->next = new;
+			skb = new;
+			bytelimit -= skb->len;
+			cnt++;
+			/* One problem here is it is difficult to
+			 * requeue the "new" dequeued skb, e.g. in
+			 * case of GSO, thus a "normal" packet can
+			 * have a GSO packet on its ->next ptr.
+			 *
+			 * Requeue is difficult because if requeuing
+			 * on q->gso_skb, then a second requeue can
+			 * happen from sch_direct_xmit e.g. if driver
+			 * returns NETDEV_TX_BUSY, which would
+			 * overwrite this requeue.
+			 */
+		}
+	} while (new && --limit && (bytelimit > 0));
+	skb = head;
+
+	return skb;
+}
+
+/* Note that dequeue_skb can possibly return a SKB list (via skb->next).
+ * A requeued skb (via q->gso_skb) can also be a SKB list.
+ */
 static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
 {
 	struct sk_buff *skb = q->gso_skb;
@@ -71,9 +132,9 @@ static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
 			skb = NULL;
 	} else {
 		if (!(q->flags & TCQ_F_ONETXQUEUE) || !netif_xmit_frozen_or_stopped(txq)) {
-			skb = q->dequeue(q);
-			if (skb)
-				skb = validate_xmit_skb(skb, qdisc_dev(q));
+			skb = qdisc_dequeue_validate(q);
+			if (skb && qdisc_may_bulk(q, skb))
+				skb = qdisc_bulk_dequeue_skb(q, skb);
 		}
 	}
 

^ permalink raw reply related


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