* Re: [PATCH] drivers: net: wireless: Add include guards
From: Kalle Valo @ 2014-08-24 12:01 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: John W. Linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1408710414-13810-1-git-send-email-linux@rasmusvillemoes.dk>
Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
> The files ray_cs.h and rayctl.h both contain two thirds of what
> appears to be an include guard using the macro name RAYLINK_H (both
> lack the #define). Since RAYLINK_H is not defined anywhere, the
> guards using different macro names.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> drivers/net/wireless/ray_cs.h | 5 +++--
> drivers/net/wireless/rayctl.h | 5 +++--
As this a change to ray_cs driver, please use prefix "ray_cs:" and drop
that "drivers: net: wireless:".
--
Kalle Valo
^ permalink raw reply
* Re: [patch net-next RFC 03/12] net: introduce generic switch devices support
From: Thomas Graf @ 2014-08-24 11:46 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, dev, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye
In-Reply-To: <1408637945-10390-4-git-send-email-jiri@resnulli.us>
On 08/21/14 at 06:18pm, Jiri Pirko wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 39294b9..8b5d14c 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -49,6 +49,8 @@
>
> #include <linux/netdev_features.h>
> #include <linux/neighbour.h>
> +#include <linux/sw_flow.h>
> +
> #include <uapi/linux/netdevice.h>
>
> struct netpoll_info;
> @@ -997,6 +999,24 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
> + * 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.
I asume you are planning to add an additional expandable struct
paramter to handle insertion parameters when the first is introduced
to avoid requiring to touch every driver every time.
> +/**
> + * swdev_flow_insert - Insert a flow into switch
> + * @dev: port device
> + * @flow: flow descriptor
> + *
> + * Insert a flow into switch this port is part of.
> + */
> +int swdev_flow_insert(struct net_device *dev, const struct sw_flow *flow)
> +{
> + const struct net_device_ops *ops = dev->netdev_ops;
> +
> + print_flow(flow, dev, "insert");
> + if (!ops->ndo_swdev_flow_insert)
> + return -EOPNOTSUPP;
> + WARN_ON(!ops->ndo_swdev_get_id);
> + BUG_ON(!flow->actions);
> + return ops->ndo_swdev_flow_insert(dev, flow);
> +}
> +EXPORT_SYMBOL(swdev_flow_insert);
Splitting the flow specific API into a separate file (maybe
swdev_flow.c?) might help resolve some of the concerns around the
focus on flows. It would make it clear that it's one of multiple
models to be supported.
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Thomas Graf @ 2014-08-24 11:32 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, Jiri Pirko,
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: <53F8CAB9.8080407-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 08/23/14 at 10:09am, John Fastabend wrote:
> Right. I think this is basically what Jiri and I discussed when he
> originally posted the series. For my use cases this is one of the
> more interesting pieces. If no one else is looking at it I can try
> it on some of the already existing open source drivers that have some
> very simple support for ingress flow tables read flow director.
Awesome. I'm definitely very interested in helping out on this part
as well.
> Thanks. This is exactly what I was trying to hint at and why the
> optimization can not be done in the driver. The driver shouldn't
> have to know about the cost models of SW vs HW rules or how to
> break up rules into sets of complimentary hw/sw rules.
That's an excellent summary of what I wanted to say.
> the other thing I've been thinking about is how to handle hardware
> with multiple flow tables. We could let the driver handle this
> but if I ever want to employ a new optimization strategy then I
> need to rewrite the driver. To me this looks a lot like policy
> which should not be driven by the kernel. We can probably ignore
> this case for the moment until we get some of the other things
> addressed.
Agreed, this sounds like something to handle a bit later.
It is potentially very interesting as it would allow to offload at
least partial pipelines but it obviously adds a new dimension to
the API. I strongly feel that the API as proposed could be extended
in this direction though. It will require a notion of tables for
swdev_flow_insert() and we'll likely need an API to set default
table policies although that is likely even needed for single table
support. We might also have to introduce a concept of bundles at
some point to provide atomic updates across multiple tables for
consistency.
> IMO I think extending the API is the easiest route but the best
> way to resolve this is to try and write the code. I'll take a
> stab at it next week.
I'm absolutely interested in writing code for this as well. If we
can find consensus on merging at least the core API bits in some
form then that would allow for more people to get involved. Maybe
we can skip the OVS bits in the first merge and continue that work
in a separate git tree. I'm also definitely very interested in hearing
Pravin's and Jesse's thoughts on the overall API ;-)
John's flow director API replacement idea can definitely serve as an
excellent first in-tree consumer as it looks even simpler.
> by the way Jiri I think the patches are a great start.
+1
^ permalink raw reply
* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Michael S. Tsirkin @ 2014-08-24 11:14 UTC (permalink / raw)
To: Pankaj Gupta
Cc: Jason Wang, Jiri Pirko, linux-kernel, netdev, davem, dgibson,
vfalico, edumazet, vyasevic, hkchu, wuzhy, xemul, therbert,
bhutchings, xii, stephen
In-Reply-To: <175719847.27217377.1408708342392.JavaMail.zimbra@redhat.com>
On Fri, Aug 22, 2014 at 07:52:22AM -0400, Pankaj Gupta wrote:
>
> >
> > On 08/20/2014 07:17 PM, Michael S. Tsirkin wrote:
> > > On Wed, Aug 20, 2014 at 12:58:17PM +0200, Jiri Pirko wrote:
> > >> > Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
> > >>> > > This patch publishes maximum number of tun/tap queues allocated as a
> > >>> > > read_only module parameter which a user space application like
> > >>> > > libvirt
> > >>> > > can make use of to limit maximum number of queues. Value of read_only
> > >>> > > module parameter can be writable only at module load time. If no
> > >>> > > value is set
> > >>> > > at module load time a default value 256 is used which is equal to
> > >>> > > maximum number
> > >>> > > of vCPUS allowed by KVM.
> > >>> > >
> > >>> > > Administrator can specify maximum number of queues only at the driver
> > >>> > > module load time.
> > >>> > >
> > >>> > >Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > >>> > >---
> > >>> > > drivers/net/tun.c | 13 +++++++++++--
> > >>> > > 1 files changed, 11 insertions(+), 2 deletions(-)
> > >>> > >
> > >>> > >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > >>> > >index acaaf67..1f518e2 100644
> > >>> > >--- a/drivers/net/tun.c
> > >>> > >+++ b/drivers/net/tun.c
> > >>> > >@@ -119,6 +119,9 @@ struct tap_filter {
> > >>> > >
> > >>> > > #define TUN_FLOW_EXPIRE (3 * HZ)
> > >>> > >
> > >>> > >+static int max_tap_queues = MAX_TAP_QUEUES;
> > >>> > >+module_param(max_tap_queues, int, S_IRUGO);
> > >> >
> > >> > Please do not introduce new module paramaters. Please other ways to
> > >> > interchange values with userspace.
> > > I suggested this initially, but thinking more about it, I agree.
> > >
> > > It's a global limit (necessary to limit memory utilization by
> > > userspace), but it should be possible to change it
> > > after module load.
> >
> > How about pass this limit through ifr during TUNSETIFF, then
> > alloc_netdev_mq() can use this limit.
>
> Any other ideas/comments from the experts. Or shall I re-repost other patches
> in the series except this patch until we agree on one.
>
> >
It's kind of useless without a way for userspace to discover
how many queues it can create, no?
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Thomas Graf @ 2014-08-24 11:12 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Scott Feldman, John Fastabend, Jiri Pirko, netdev, davem, nhorman,
andy, dborkman, ogerlitz, jesse, pshelar, azhou, ben, stephen,
jeffrey.t.kirsher, vyasevic, xiyou.wangcong, john.r.fastabend,
edumazet, f.fainelli, roopa, linville, dev, jasowang, ebiederm,
nicolas.dichtel, ryazanov.s.a, buytenh, aviadr, nbd,
alexei.starovoitov, Neil.Jerram, ronye
In-Reply-To: <53F9459B.2070801@mojatatu.com>
On 08/23/14 at 09:53pm, Jamal Hadi Salim wrote:
> On 08/22/14 18:53, Scott Feldman wrote:
>
> Ok, Scott - now i have looked at the patches on the plane and i am
> still not convinced ;->
>
> >The intent is to use openvswitch.ko’s struct sw_flow to program hardware via the
> >ndo_swdev_flow_* ops, but otherwise be independent of OVS. So the upper layer of
> >the driver is struct sw_flow and any module above the driver can construct a struct
> >sw_flow and push it down via ndo_swdev_flow_*. So your non-OVS use-case should be
> >handled. OVS is another use-case. struct sw_flow should not be OVS-aware, but
> >rather a generic flow match/action sufficient to offload the data plane to HW.
>
>
> There is a legitimate case to be made for offloading OVS but *not*
> a basis for making it the offload interface.
> My suggestion is to make all OVS stuff a separate patchset.
> This thing needs to stand alone without OVS and we dont need
> to confuse the two.
I get what you are saying but I don't see that to be the case here. I
don't see how this series proposes the OVS case as *the* interface.
It proposes *a* interface which in this case is flow based with mask
support to accomodate the typical ntuple filter API in HW. OVS happens
to be one of the easiest to use examples as a consumer because it
already provides a flat flow representation.
That said, I already mentioned that I see a lot of value in having a
non OVS API example ASAP and I will be glad to help out John to achieve
that.
> Having said that:
> I believe in starting simple - by solving the basic functions of
> L2/3 offload first because those are well understood and fundamental.
> There is the simplicity of those network functions and then
> need to deal with tons of quarks that surround them....
> I think getting that right will help in understanding the issues and
> make this interface better. This is where i am going to focus my effort.
I thought this is exactly what is happening here. The flow key/mask
based API as proposed focuses on basic forwarding for L2-L4.
> Here's my view on flows in the patchset:
> What we need is ability to specify different types of classifiers.
> But leave L2 and 3 out of that - that should be part of the basic
> feature set.
>
> Your 15-tuple classifier should be one of those classifiers.
> This is because you *cannot possibly* have a universal classifier.
> The tc classifier/action API has got this part right. There is
> no ONE flow classifier but rather it has flexibility to add as many
> as you want.
Exactly and I never saw Jiri claim that swdev_flow_insert() would be
the only offload capability exposed by the API. I see no reason why
it could not also provide swdev_offset_match_insert() or
swdev_ebpf_insert() for the 2*next generation HW. I don't think it
makes sense to focus entirely on finding a single common denominator
and channel everything through a single function to represent all the
different generic and less generic offload capabilities. I believe
that doing so will raise the minimal HW requirements barrier HW too
much. I think we should start somewhere, learn and evolve.
> IOW:
> I should be able to specify a classifier that matches the
> definition of the openflow thing you are using. But then i should also
> be able to create one based on 32 bit value/masks, one that classifies
> strings, one that classifies metadata, my own pigeon observer
> classifier etc. And be able to attach them in combinations
> to select different things within the packet and act differently.
So essentially what you are saying is that the tc interface
(in particular cls and act) could be used as an API to achieve offloads.
Yes! I thought this was very clear and a given. I don't think that it
makes sense to force every offload API consumer through the tc interface
though. This comes back to my statements in a previous email. I don't
think we should require that all the offload decision complexity *has*
to live in the kernel. Quagga, nft, or OVS should be given an API to
influence this more directly (with the hardware complexity properly
abstracted). In-kernel users such as bridge, l3 (especially rules),
and tc itself could be handled through a cls/act derived API internally.
> Lets pick an example of the u32 classifier (or i could pick nftables).
> Using your scheme i have to incur penalties to translating u32 to your
> classifier and only achieve basic functionality; and now in addition
> i cant do 90% of my u32 features. And u32 is very implementable
> in hardware.
I don't fully understand the last claim. Given the specific ntuple
capabilities of a lot of hardware out there (let's assume a typical
5-tuple capability with N capacity for exact matches and M capacity for
wildcard matches) supporting a generic u32 offset-len-mask is not exactly
trivial at all and I don't see how you can get around converting the
generic offset into a ntuple filter *at some point* to verify if the HW
can fullfil the generic offset match request or not. Could you share
what kind of HW you regard as a minimal requirement to base the offload
API on? Personally I'm highly interested in the existing limited tuple
filters and flow directors of NICs already available and their next
successors. I think that the code that Jiri proposes and what John is
planning to do makes a lot of sense in that context.
^ permalink raw reply
* Re: [PATCH 0/3] Basic deferred TX queue flushing infrastructure.
From: David Miller @ 2014-08-24 4:38 UTC (permalink / raw)
To: netdev; +Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
In-Reply-To: <20140823.132811.751469424156827125.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sat, 23 Aug 2014 13:28:11 -0700 (PDT)
> This just adds the infrastructure, it does not actually add any
> instances of actually doing multiple ndo_start_xmit calls per
> ndo_xmit_flush invocation.
So today I was looking more into this aspect.
Like Tom Herbert has mentioned we have all the infrastructure (sort
of) already to handle a list of SKBs going down into
dev_hard_start_xmit() via the GSO handling.
But that code is funny, because it keeps the original GSO head SKB
around as a placeholder to maintain the list of segmented SKBs.
So the list walker basically walks starting at skb->next. That's
awkward for what we want to do, which is pass in an arbitrary list of
SKBs.
All it really wants that head SKB for is essentially list management,
which seems like overkill to me.
Anyways, this got me thinking that we should have something that
provides the segment list management and stop keeping that head GSO
SKB around.
Then we can make that "gso:" label list walker generic enough that we
could pass down arbitrary lists of SKBs from the qdisc_restart() path.
This list management seems to be the only reason why we keep the GSO
head SKB around after dev_gso_segment(), we should be able to free it
up early without any problems right?
I'm also thinking about whether we should hang the generic SKB list
management off of the txq or the qdisc. Right now the gso_skb thing
is in the qdisc.
Thoughts?
^ permalink raw reply
* Re: [PATCH 0/3] Basic deferred TX queue flushing infrastructure.
From: David Miller @ 2014-08-24 4:26 UTC (permalink / raw)
To: therbert; +Cc: netdev, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
In-Reply-To: <CA+mtBx_FH07PTRS8Bbuh9QoWRNgiEFd2CjEtKa2XJwh1rb5CuQ@mail.gmail.com>
From: Tom Herbert <therbert@google.com>
Date: Sat, 23 Aug 2014 20:39:05 -0700
> Interesting, but I'm still wondering about MSG_MORE idea. If that is
> passed with ndo_start_xmit than maybe a new ndo function might not be
> needed. Also, since it's advisory, we could pass it to existing
> drivers and let them decide how rather to do anything with it.
My thinking is that if we can have the flush generation logic in a
generic place, we should.
We really do not want every driver to have a unique approach to this
stuff.
^ permalink raw reply
* Re: [PATCH 0/3] Basic deferred TX queue flushing infrastructure.
From: Tom Herbert @ 2014-08-24 3:39 UTC (permalink / raw)
To: David Miller
Cc: Linux Netdev List, Jamal Hadi Salim, Hannes Frederic Sowa,
Eric Dumazet, Jeff Kirsher, Rusty Russell
In-Reply-To: <20140823.132811.751469424156827125.davem@davemloft.net>
On Sat, Aug 23, 2014 at 1:28 PM, David Miller <davem@davemloft.net> wrote:
>
>
> Over time, and specifically and more recently at the Networking
> Workshop during Kernel SUmmit in Chicago, we have discussed the idea
> of having some way to optimize transmits of multiple TX packets at
> a time.
>
> There are several areas of overhead that could be amortized with such
> schemes. One has to do with locking and transactional overhead, the
> other has to do with device specific costs.
>
> This patch set here is more aimed at device specific costs.
>
> Typically a device queues up a packet in the TX queue and then has to
> do something to have the device start processing that new entry.
> Sometimes this is composed of doing an MMIO write to a "tail"
> register, and in other cases it can involve something as expensive as
> a hypervisor call.
>
> The basic setup defined here is that when the driver supports deferred
> TX queue flushing, ndo_start_xmit should no longer perform that
> operation. Instead a new operation, ndo_xmit_flush, should do it.
>
Interesting, but I'm still wondering about MSG_MORE idea. If that is
passed with ndo_start_xmit than maybe a new ndo function might not be
needed. Also, since it's advisory, we could pass it to existing
drivers and let them decide how rather to do anything with it.
> I have converted IGB and virtio_net as example initial users. The IGB
> conversion is tested, virtio_net is not but it does compile :-)
>
> All ndo_start_xmit call sites have been abstracted behind a new helper
> called netdev_start_xmit().
>
> This just adds the infrastructure, it does not actually add any
> instances of actually doing multiple ndo_start_xmit calls per
> ndo_xmit_flush invocation.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Jamal Hadi Salim @ 2014-08-24 1:53 UTC (permalink / raw)
To: Scott Feldman, 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, Jiri Pirko,
roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
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: <464DB0A8-0073-4CE0-9483-0F36B73A53A1-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
On 08/22/14 18:53, Scott Feldman wrote:
Ok, Scott - now i have looked at the patches on the plane and i am
still not convinced ;->
> The intent is to use openvswitch.ko’s struct sw_flow to program hardware via the
>ndo_swdev_flow_* ops, but otherwise be independent of OVS. So the upper layer of
>the driver is struct sw_flow and any module above the driver can construct a struct
>sw_flow and push it down via ndo_swdev_flow_*. So your non-OVS use-case should be
>handled. OVS is another use-case. struct sw_flow should not be OVS-aware, but
>rather a generic flow match/action sufficient to offload the data plane to HW.
There is a legitimate case to be made for offloading OVS but *not*
a basis for making it the offload interface.
My suggestion is to make all OVS stuff a separate patchset.
This thing needs to stand alone without OVS and we dont need
to confuse the two.
Having said that:
I believe in starting simple - by solving the basic functions of
L2/3 offload first because those are well understood and fundamental.
There is the simplicity of those network functions and then
need to deal with tons of quarks that surround them....
I think getting that right will help in understanding the issues and
make this interface better. This is where i am going to focus my effort.
Here's my view on flows in the patchset:
What we need is ability to specify different types of classifiers.
But leave L2 and 3 out of that - that should be part of the basic
feature set.
Your 15-tuple classifier should be one of those classifiers.
This is because you *cannot possibly* have a universal classifier.
The tc classifier/action API has got this part right. There is
no ONE flow classifier but rather it has flexibility to add as many
as you want.
IOW:
I should be able to specify a classifier that matches the
definition of the openflow thing you are using. But then i should also
be able to create one based on 32 bit value/masks, one that classifies
strings, one that classifies metadata, my own pigeon observer
classifier etc. And be able to attach them in combinations
to select different things within the packet and act differently.
Lets pick an example of the u32 classifier (or i could pick nftables).
Using your scheme i have to incur penalties to translating u32 to your
classifier and only achieve basic functionality; and now in addition
i cant do 90% of my u32 features. And u32 is very implementable
in hardware.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 5/8] i40e: Fix TSO and hw checksums for non-accelerated vlan packets.
From: Vlad Yasevich @ 2014-08-24 1:13 UTC (permalink / raw)
To: David Miller
Cc: vyasevic, linux.nics, e1000-devel, netdev, bruce.w.allan,
jesse.brandeburg, john.ronciak
In-Reply-To: <20140823.114357.1040666365885664477.davem@davemloft.net>
On 08/23/2014 02:43 PM, David Miller wrote:
> From: vyasevich@gmail.com
> Date: Fri, 22 Aug 2014 22:17:07 -0400
>
>> @@ -2295,7 +2295,7 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
>> goto out_drop;
>>
>> /* obtain protocol of skb */
>> - protocol = skb->protocol;
>> + protocol = get_vlan_protocol(skb);
>
> I don't think this even compiles.
>
> It's "vlan_get_protocol" not "get_vlan_protocol".
>
Yes. I notice this one as well this morning, but didn't have time to fix.
Apologies.
-vlad
------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds. Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH 1/3] net: Add ops->ndo_xmit_flush()
From: David Miller @ 2014-08-24 0:19 UTC (permalink / raw)
To: alexander.duyck
Cc: netdev, therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
In-Reply-To: <53F9251A.80005@gmail.com>
From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Sat, 23 Aug 2014 16:34:50 -0700
> On 08/23/2014 01:28 PM, David Miller wrote:
>> @@ -3358,6 +3372,27 @@ int __init dev_proc_init(void);
>> #define dev_proc_init() 0
>> #endif
>>
>> +static inline netdev_tx_t __netdev_start_xmit(const struct net_device_ops *ops,
>> + struct sk_buff *skb, struct net_device *dev)
>> +{
>> + netdev_tx_t ret;
>> + u16 q;
>> +
>> + q = skb->queue_mapping;
>> + ret = ops->ndo_start_xmit(skb, dev);
>> + if (ops->ndo_xmit_flush)
>> + ops->ndo_xmit_flush(dev, q);
>> +
>> + return ret;
>> +}
>> +
>
> What about the case of ndo_start_xmit returning something like
> NETDEV_TX_BUSY? I am pretty sure you shouldn't be flushing unless
> something has been enqueued. You might want to add a new return that
> specified that a frame has been enqueued but not flushed and then start
> down the ndo_xmit_flush path. Maybe something like NETDEV_TX_DEFERRED.
>
> You might even want to have a return from ndo_xmit_flush just to cover
> any oddball cases like a lockless Tx where we might not be able to flush
> because the queue is already being flushed by another entity.
Indeed, the code as-is isn't correct and should guard the flush
with a check of the 'ret' value.
I don't think LLTX drivers would be able to utilize the flush
facility, which is even more incentive to not use LLTX.
^ permalink raw reply
* Re: [PATCH RFC net-next 2/2] net: skbuff: do not allocate emergency memory if flags is not __GFP_MEMALLOC
From: Eric Dumazet @ 2014-08-23 23:53 UTC (permalink / raw)
To: Govindarajulu Varadarajan; +Cc: netdev, davem, edumazet, mgorman
In-Reply-To: <1408829839-20742-3-git-send-email-_govind@gmx.com>
On Sun, 2014-08-24 at 03:07 +0530, Govindarajulu Varadarajan wrote:
> It is possible that nc->frag.page is previously allocated from emergency memory.
> For new alloc call, if the flags does not contain __GFP_MEMALLOC, do not
> return pfmemalloc memory.
>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
> ---
> net/core/skbuff.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 51a3328..792ce89 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -364,6 +364,13 @@ recycle:
> atomic_set(&nc->frag.page->_count, NETDEV_PAGECNT_MAX_BIAS);
> nc->pagecnt_bias = NETDEV_PAGECNT_MAX_BIAS;
> nc->frag.offset = 0;
> + } else if (nc->frag.page->pfmemalloc && !(gfp_mask & __GFP_MEMALLOC)) {
> + if (atomic_sub_and_test(nc->pagecnt_bias,
> + &nc->frag.page->_count)) {
> + atomic_set(&nc->frag.page->_count, 1);
> + kfree(page_address(nc->frag.page));
No idea of what you think you do.
How have you tested this patch ?
> + }
> + goto refill;
> }
>
> if (nc->frag.offset + fragsz > nc->frag.size) {
^ permalink raw reply
* Re: [PATCH RFC net-next 1/2] net: skbuff: propagate pfmemalloc to skb
From: Eric Dumazet @ 2014-08-23 23:51 UTC (permalink / raw)
To: Govindarajulu Varadarajan; +Cc: netdev, davem, edumazet, mgorman
In-Reply-To: <1408829839-20742-2-git-send-email-_govind@gmx.com>
On Sun, 2014-08-24 at 03:07 +0530, Govindarajulu Varadarajan wrote:
> In __netdev_alloc_skb, __netdev_alloc_frag might have allocated page with
> flag __GFP_MEMALLOC. But we do not propagate it to the skb.
>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
> ---
> net/core/skbuff.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 163b673..51a3328 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -425,8 +425,14 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
>
> if (likely(data)) {
> skb = build_skb(data, fragsz);
> - if (unlikely(!skb))
> + if (unlikely(!skb)) {
> put_page(virt_to_head_page(data));
> + } else {
> + struct page *page;
> +
> + page = virt_to_head_page(data);
> + skb_propagate_pfmemalloc(page, skb);
> + }
> }
> } else {
> skb = __alloc_skb(length + NET_SKB_PAD, gfp_mask,
Oh well...
We made everything we could to avoid touching the page head for every
frag allocation.
Just cache page->pfmemalloc into nc->frag.pfmemalloc
Think of how often asymmetric alloc/free happen in networking stack and
why we use NETDEV_PAGECNT_MAX_BIAS trick.
Thanks
^ permalink raw reply
* Re: [PATCH 1/3] net: Add ops->ndo_xmit_flush()
From: Alexander Duyck @ 2014-08-23 23:34 UTC (permalink / raw)
To: David Miller, netdev
Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
In-Reply-To: <20140823.132823.531609193955178433.davem@davemloft.net>
On 08/23/2014 01:28 PM, David Miller wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 7e2b0b8..1d05932 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -782,6 +782,19 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
> * (can also return NETDEV_TX_LOCKED iff NETIF_F_LLTX)
> * Required can not be NULL.
> *
> + * void (*ndo_xmit_flush)(struct net_device *dev, u16 queue);
> + * A driver implements this function when it wishes to support
> + * deferred TX queue flushing. The idea is that the expensive
> + * operation to trigger TX queue processing can be done after
> + * N calls to ndo_start_xmit rather than being done every single
> + * time. In this regime ndo_start_xmit will be called one or more
> + * times, and then a final ndo_xmit_flush call will be made to
> + * have the driver tell the device about the new pending TX queue
> + * entries. The kernel keeps track of which queues need flushing
> + * by monitoring skb->queue_mapping of the packets it submits to
> + * ndo_start_xmit. This is the queue value that will be passed
> + * to ndo_xmit_flush.
> + *
> * u16 (*ndo_select_queue)(struct net_device *dev, struct sk_buff *skb,
> * void *accel_priv, select_queue_fallback_t fallback);
> * Called to decide which queue to when device supports multiple
> @@ -1005,6 +1018,7 @@ struct net_device_ops {
> int (*ndo_stop)(struct net_device *dev);
> netdev_tx_t (*ndo_start_xmit) (struct sk_buff *skb,
> struct net_device *dev);
> + void (*ndo_xmit_flush)(struct net_device *dev, u16 queue);
> u16 (*ndo_select_queue)(struct net_device *dev,
> struct sk_buff *skb,
> void *accel_priv,
> @@ -3358,6 +3372,27 @@ int __init dev_proc_init(void);
> #define dev_proc_init() 0
> #endif
>
> +static inline netdev_tx_t __netdev_start_xmit(const struct net_device_ops *ops,
> + struct sk_buff *skb, struct net_device *dev)
> +{
> + netdev_tx_t ret;
> + u16 q;
> +
> + q = skb->queue_mapping;
> + ret = ops->ndo_start_xmit(skb, dev);
> + if (ops->ndo_xmit_flush)
> + ops->ndo_xmit_flush(dev, q);
> +
> + return ret;
> +}
> +
What about the case of ndo_start_xmit returning something like
NETDEV_TX_BUSY? I am pretty sure you shouldn't be flushing unless
something has been enqueued. You might want to add a new return that
specified that a frame has been enqueued but not flushed and then start
down the ndo_xmit_flush path. Maybe something like NETDEV_TX_DEFERRED.
You might even want to have a return from ndo_xmit_flush just to cover
any oddball cases like a lockless Tx where we might not be able to flush
because the queue is already being flushed by another entity.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 0/3] Basic deferred TX queue flushing infrastructure.
From: Alexander Duyck @ 2014-08-23 23:25 UTC (permalink / raw)
To: David Miller, netdev
Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
In-Reply-To: <20140823.132811.751469424156827125.davem@davemloft.net>
On 08/23/2014 01:28 PM, David Miller wrote:
>
> Over time, and specifically and more recently at the Networking
> Workshop during Kernel SUmmit in Chicago, we have discussed the idea
> of having some way to optimize transmits of multiple TX packets at
> a time.
>
> There are several areas of overhead that could be amortized with such
> schemes. One has to do with locking and transactional overhead, the
> other has to do with device specific costs.
>
> This patch set here is more aimed at device specific costs.
>
> Typically a device queues up a packet in the TX queue and then has to
> do something to have the device start processing that new entry.
> Sometimes this is composed of doing an MMIO write to a "tail"
> register, and in other cases it can involve something as expensive as
> a hypervisor call.
The MMIO call isn't an issue until you encounter a locked operation, at
least on x86 architecture. So this often shows up in perf traces as a
hit on the qdisc lock right after completing a transmit. I've seen it
at around 20% of CPU utilization when I was doing routing work with ixgbe.
Thanks,
Alex
^ permalink raw reply
* [PATCH RFC net-next 2/2] net: skbuff: do not allocate emergency memory if flags is not __GFP_MEMALLOC
From: Govindarajulu Varadarajan @ 2014-08-23 21:37 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, mgorman, Govindarajulu Varadarajan
In-Reply-To: <1408829839-20742-1-git-send-email-_govind@gmx.com>
It is possible that nc->frag.page is previously allocated from emergency memory.
For new alloc call, if the flags does not contain __GFP_MEMALLOC, do not
return pfmemalloc memory.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
net/core/skbuff.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 51a3328..792ce89 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -364,6 +364,13 @@ recycle:
atomic_set(&nc->frag.page->_count, NETDEV_PAGECNT_MAX_BIAS);
nc->pagecnt_bias = NETDEV_PAGECNT_MAX_BIAS;
nc->frag.offset = 0;
+ } else if (nc->frag.page->pfmemalloc && !(gfp_mask & __GFP_MEMALLOC)) {
+ if (atomic_sub_and_test(nc->pagecnt_bias,
+ &nc->frag.page->_count)) {
+ atomic_set(&nc->frag.page->_count, 1);
+ kfree(page_address(nc->frag.page));
+ }
+ goto refill;
}
if (nc->frag.offset + fragsz > nc->frag.size) {
--
2.1.0
^ permalink raw reply related
* [PATCH RFC net-next 1/2] net: skbuff: propagate pfmemalloc to skb
From: Govindarajulu Varadarajan @ 2014-08-23 21:37 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, mgorman, Govindarajulu Varadarajan
In-Reply-To: <1408829839-20742-1-git-send-email-_govind@gmx.com>
In __netdev_alloc_skb, __netdev_alloc_frag might have allocated page with
flag __GFP_MEMALLOC. But we do not propagate it to the skb.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
net/core/skbuff.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 163b673..51a3328 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -425,8 +425,14 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
if (likely(data)) {
skb = build_skb(data, fragsz);
- if (unlikely(!skb))
+ if (unlikely(!skb)) {
put_page(virt_to_head_page(data));
+ } else {
+ struct page *page;
+
+ page = virt_to_head_page(data);
+ skb_propagate_pfmemalloc(page, skb);
+ }
}
} else {
skb = __alloc_skb(length + NET_SKB_PAD, gfp_mask,
--
2.1.0
^ permalink raw reply related
* [PATCH RFC net-next 0/2] Fix pfmemalloc in __netdev_alloc_skb
From: Govindarajulu Varadarajan @ 2014-08-23 21:37 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, mgorman, Govindarajulu Varadarajan
Hi
After the introduction of skb->pfmemalloc in
c93bdd0e0:(allow skb allocation to use PFMEMALLOC reserves), skb->pfmemalloc
should be set to true if buff is allocated from reserve memory.
But in a1c7fff7e(netdev_alloc_skb() use build_skb()), netdev_alloc_skb() does
not set skb->pfmemalloc if the page is allocated from reserve memory.
Also if the page in netdev_alloc_cache->frag is allocated from reserve memory,
and if the next call to __netdev_alloc_frag does not have __GFP_MEMALLOC in
flags, we should not return a reserved memory. Because the reserve memory could
be sitting in receive queue waiting for packet to arrive. We should release
the reserve memory as soon as possible.
Is my understanding correct?
Govindarajulu Varadarajan (2):
net: skbuff: propagate pfmemalloc to skb
net: skbuff: do not allocate emergency memory if flags is not
__GFP_MEMALLOC
net/core/skbuff.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
--
2.1.0
^ permalink raw reply
* [PATCH 3/3] virtio_net: Support netdev_ops->ndo_xmit_flush()
From: David Miller @ 2014-08-23 20:28 UTC (permalink / raw)
To: netdev; +Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/virtio_net.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 59caa06..6242108 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -934,7 +934,6 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}
- virtqueue_kick(sq->vq);
/* Don't wait up for transmitted skbs to be freed. */
skb_orphan(skb);
@@ -957,6 +956,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
+static void xmit_flush(struct net_device *dev, u16 qnum)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ struct send_queue *sq = &vi->sq[qnum];
+
+ virtqueue_kick(sq->vq);
+}
+
/*
* Send command via the control virtqueue and check status. Commands
* supported by the hypervisor, as indicated by feature bits, should
@@ -1386,6 +1393,7 @@ static const struct net_device_ops virtnet_netdev = {
.ndo_open = virtnet_open,
.ndo_stop = virtnet_close,
.ndo_start_xmit = start_xmit,
+ .ndo_xmit_flush = xmit_flush,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = virtnet_set_mac_address,
.ndo_set_rx_mode = virtnet_set_rx_mode,
--
1.7.11.7
^ permalink raw reply related
* [PATCH 2/3] igb: Support netdev_ops->ndo_xmit_flush()
From: David Miller @ 2014-08-23 20:28 UTC (permalink / raw)
To: netdev; +Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/intel/igb/igb_main.c | 35 +++++++++++++++++++++----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index cb14bbd..b9c020a 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -136,6 +136,7 @@ static void igb_update_phy_info(unsigned long);
static void igb_watchdog(unsigned long);
static void igb_watchdog_task(struct work_struct *);
static netdev_tx_t igb_xmit_frame(struct sk_buff *skb, struct net_device *);
+static void igb_xmit_flush(struct net_device *netdev, u16 queue);
static struct rtnl_link_stats64 *igb_get_stats64(struct net_device *dev,
struct rtnl_link_stats64 *stats);
static int igb_change_mtu(struct net_device *, int);
@@ -2075,6 +2076,7 @@ static const struct net_device_ops igb_netdev_ops = {
.ndo_open = igb_open,
.ndo_stop = igb_close,
.ndo_start_xmit = igb_xmit_frame,
+ .ndo_xmit_flush = igb_xmit_flush,
.ndo_get_stats64 = igb_get_stats64,
.ndo_set_rx_mode = igb_set_rx_mode,
.ndo_set_mac_address = igb_set_mac,
@@ -4915,13 +4917,6 @@ static void igb_tx_map(struct igb_ring *tx_ring,
tx_ring->next_to_use = i;
- writel(i, tx_ring->tail);
-
- /* we need this if more than one processor can write to our tail
- * at a time, it synchronizes IO on IA64/Altix systems
- */
- mmiowb();
-
return;
dma_error:
@@ -5057,17 +5052,20 @@ out_drop:
return NETDEV_TX_OK;
}
-static inline struct igb_ring *igb_tx_queue_mapping(struct igb_adapter *adapter,
- struct sk_buff *skb)
+static struct igb_ring *__igb_tx_queue_mapping(struct igb_adapter *adapter, unsigned int r_idx)
{
- unsigned int r_idx = skb->queue_mapping;
-
if (r_idx >= adapter->num_tx_queues)
r_idx = r_idx % adapter->num_tx_queues;
return adapter->tx_ring[r_idx];
}
+static inline struct igb_ring *igb_tx_queue_mapping(struct igb_adapter *adapter,
+ struct sk_buff *skb)
+{
+ return __igb_tx_queue_mapping(adapter, skb->queue_mapping);
+}
+
static netdev_tx_t igb_xmit_frame(struct sk_buff *skb,
struct net_device *netdev)
{
@@ -5096,6 +5094,21 @@ static netdev_tx_t igb_xmit_frame(struct sk_buff *skb,
return igb_xmit_frame_ring(skb, igb_tx_queue_mapping(adapter, skb));
}
+static void igb_xmit_flush(struct net_device *netdev, u16 queue)
+{
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ struct igb_ring *tx_ring;
+
+ tx_ring = __igb_tx_queue_mapping(adapter, queue);
+
+ writel(tx_ring->next_to_use, tx_ring->tail);
+
+ /* we need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
+}
+
/**
* igb_tx_timeout - Respond to a Tx Hang
* @netdev: network interface device structure
--
1.7.11.7
^ permalink raw reply related
* [PATCH 1/3] net: Add ops->ndo_xmit_flush()
From: David Miller @ 2014-08-23 20:28 UTC (permalink / raw)
To: netdev; +Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/wan/dlci.c | 2 +-
drivers/usb/gadget/function/f_ncm.c | 2 +-
include/linux/netdevice.h | 35 +++++++++++++++++++++++++++++++++++
net/atm/mpc.c | 2 +-
net/core/dev.c | 5 ++---
net/core/netpoll.c | 3 +--
net/core/pktgen.c | 4 +---
net/packet/af_packet.c | 3 +--
net/sched/sch_teql.c | 3 +--
9 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wan/dlci.c b/drivers/net/wan/dlci.c
index 43c9960..81b22a1 100644
--- a/drivers/net/wan/dlci.c
+++ b/drivers/net/wan/dlci.c
@@ -193,7 +193,7 @@ static netdev_tx_t dlci_transmit(struct sk_buff *skb, struct net_device *dev)
struct dlci_local *dlp = netdev_priv(dev);
if (skb)
- dlp->slave->netdev_ops->ndo_start_xmit(skb, dlp->slave);
+ netdev_start_xmit(skb, dlp->slave);
return NETDEV_TX_OK;
}
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index bcdc882..cb5d646 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1101,7 +1101,7 @@ static void ncm_tx_tasklet(unsigned long data)
/* Only send if data is available. */
if (ncm->skb_tx_data) {
ncm->timer_force_tx = true;
- ncm->netdev->netdev_ops->ndo_start_xmit(NULL, ncm->netdev);
+ netdev_start_xmit(NULL, ncm->netdev);
ncm->timer_force_tx = false;
}
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7e2b0b8..1d05932 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -782,6 +782,19 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
* (can also return NETDEV_TX_LOCKED iff NETIF_F_LLTX)
* Required can not be NULL.
*
+ * void (*ndo_xmit_flush)(struct net_device *dev, u16 queue);
+ * A driver implements this function when it wishes to support
+ * deferred TX queue flushing. The idea is that the expensive
+ * operation to trigger TX queue processing can be done after
+ * N calls to ndo_start_xmit rather than being done every single
+ * time. In this regime ndo_start_xmit will be called one or more
+ * times, and then a final ndo_xmit_flush call will be made to
+ * have the driver tell the device about the new pending TX queue
+ * entries. The kernel keeps track of which queues need flushing
+ * by monitoring skb->queue_mapping of the packets it submits to
+ * ndo_start_xmit. This is the queue value that will be passed
+ * to ndo_xmit_flush.
+ *
* u16 (*ndo_select_queue)(struct net_device *dev, struct sk_buff *skb,
* void *accel_priv, select_queue_fallback_t fallback);
* Called to decide which queue to when device supports multiple
@@ -1005,6 +1018,7 @@ struct net_device_ops {
int (*ndo_stop)(struct net_device *dev);
netdev_tx_t (*ndo_start_xmit) (struct sk_buff *skb,
struct net_device *dev);
+ void (*ndo_xmit_flush)(struct net_device *dev, u16 queue);
u16 (*ndo_select_queue)(struct net_device *dev,
struct sk_buff *skb,
void *accel_priv,
@@ -3358,6 +3372,27 @@ int __init dev_proc_init(void);
#define dev_proc_init() 0
#endif
+static inline netdev_tx_t __netdev_start_xmit(const struct net_device_ops *ops,
+ struct sk_buff *skb, struct net_device *dev)
+{
+ netdev_tx_t ret;
+ u16 q;
+
+ q = skb->queue_mapping;
+ ret = ops->ndo_start_xmit(skb, dev);
+ if (ops->ndo_xmit_flush)
+ ops->ndo_xmit_flush(dev, q);
+
+ return ret;
+}
+
+static inline netdev_tx_t netdev_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+
+ return __netdev_start_xmit(ops, skb, dev);
+}
+
int netdev_class_create_file_ns(struct class_attribute *class_attr,
const void *ns);
void netdev_class_remove_file_ns(struct class_attribute *class_attr,
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index e8e0e7a..d662da1 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -599,7 +599,7 @@ static netdev_tx_t mpc_send_packet(struct sk_buff *skb,
}
non_ip:
- return mpc->old_ops->ndo_start_xmit(skb, dev);
+ return __netdev_start_xmit(mpc->old_ops, skb, dev);
}
static int atm_mpoa_vcc_attach(struct atm_vcc *vcc, void __user *arg)
diff --git a/net/core/dev.c b/net/core/dev.c
index 1421dad..6cf06a8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2602,7 +2602,6 @@ EXPORT_SYMBOL(netif_skb_features);
int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq)
{
- const struct net_device_ops *ops = dev->netdev_ops;
int rc = NETDEV_TX_OK;
unsigned int skb_len;
@@ -2667,7 +2666,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
skb_len = skb->len;
trace_net_dev_start_xmit(skb, dev);
- rc = ops->ndo_start_xmit(skb, dev);
+ rc = netdev_start_xmit(skb, dev);
trace_net_dev_xmit(skb, rc, dev, skb_len);
if (rc == NETDEV_TX_OK)
txq_trans_update(txq);
@@ -2686,7 +2685,7 @@ gso:
skb_len = nskb->len;
trace_net_dev_start_xmit(nskb, dev);
- rc = ops->ndo_start_xmit(nskb, dev);
+ rc = netdev_start_xmit(nskb, dev);
trace_net_dev_xmit(nskb, rc, dev, skb_len);
if (unlikely(rc != NETDEV_TX_OK)) {
if (rc & ~NETDEV_TX_MASK)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 907fb5e..a5ad068 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -72,7 +72,6 @@ module_param(carrier_timeout, uint, 0644);
static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq)
{
- const struct net_device_ops *ops = dev->netdev_ops;
int status = NETDEV_TX_OK;
netdev_features_t features;
@@ -92,7 +91,7 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
skb->vlan_tci = 0;
}
- status = ops->ndo_start_xmit(skb, dev);
+ status = netdev_start_xmit(skb, dev);
if (status == NETDEV_TX_OK)
txq_trans_update(txq);
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 8b849dd..83e2b4b 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3285,8 +3285,6 @@ static void pktgen_wait_for_skb(struct pktgen_dev *pkt_dev)
static void pktgen_xmit(struct pktgen_dev *pkt_dev)
{
struct net_device *odev = pkt_dev->odev;
- netdev_tx_t (*xmit)(struct sk_buff *, struct net_device *)
- = odev->netdev_ops->ndo_start_xmit;
struct netdev_queue *txq;
u16 queue_map;
int ret;
@@ -3339,7 +3337,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
goto unlock;
}
atomic_inc(&(pkt_dev->skb->users));
- ret = (*xmit)(pkt_dev->skb, odev);
+ ret = netdev_start_xmit(pkt_dev->skb, odev);
switch (ret) {
case NETDEV_TX_OK:
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 93896d2..0dfa990 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -240,7 +240,6 @@ static void __fanout_link(struct sock *sk, struct packet_sock *po);
static int packet_direct_xmit(struct sk_buff *skb)
{
struct net_device *dev = skb->dev;
- const struct net_device_ops *ops = dev->netdev_ops;
netdev_features_t features;
struct netdev_queue *txq;
int ret = NETDEV_TX_BUSY;
@@ -262,7 +261,7 @@ static int packet_direct_xmit(struct sk_buff *skb)
HARD_TX_LOCK(dev, txq, smp_processor_id());
if (!netif_xmit_frozen_or_drv_stopped(txq)) {
- ret = ops->ndo_start_xmit(skb, dev);
+ ret = netdev_start_xmit(skb, dev);
if (ret == NETDEV_TX_OK)
txq_trans_update(txq);
}
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index bd33793..64cd93c 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -301,7 +301,6 @@ restart:
do {
struct net_device *slave = qdisc_dev(q);
struct netdev_queue *slave_txq = netdev_get_tx_queue(slave, 0);
- const struct net_device_ops *slave_ops = slave->netdev_ops;
if (slave_txq->qdisc_sleeping != q)
continue;
@@ -317,7 +316,7 @@ restart:
unsigned int length = qdisc_pkt_len(skb);
if (!netif_xmit_frozen_or_stopped(slave_txq) &&
- slave_ops->ndo_start_xmit(skb, slave) == NETDEV_TX_OK) {
+ netdev_start_xmit(skb, slave) == NETDEV_TX_OK) {
txq_trans_update(slave_txq);
__netif_tx_unlock(slave_txq);
master->slaves = NEXT_SLAVE(q);
--
1.7.11.7
^ permalink raw reply related
* [PATCH 0/3] Basic deferred TX queue flushing infrastructure.
From: David Miller @ 2014-08-23 20:28 UTC (permalink / raw)
To: netdev; +Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty
Over time, and specifically and more recently at the Networking
Workshop during Kernel SUmmit in Chicago, we have discussed the idea
of having some way to optimize transmits of multiple TX packets at
a time.
There are several areas of overhead that could be amortized with such
schemes. One has to do with locking and transactional overhead, the
other has to do with device specific costs.
This patch set here is more aimed at device specific costs.
Typically a device queues up a packet in the TX queue and then has to
do something to have the device start processing that new entry.
Sometimes this is composed of doing an MMIO write to a "tail"
register, and in other cases it can involve something as expensive as
a hypervisor call.
The basic setup defined here is that when the driver supports deferred
TX queue flushing, ndo_start_xmit should no longer perform that
operation. Instead a new operation, ndo_xmit_flush, should do it.
I have converted IGB and virtio_net as example initial users. The IGB
conversion is tested, virtio_net is not but it does compile :-)
All ndo_start_xmit call sites have been abstracted behind a new helper
called netdev_start_xmit().
This just adds the infrastructure, it does not actually add any
instances of actually doing multiple ndo_start_xmit calls per
ndo_xmit_flush invocation.
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH v5 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
From: Richard Cochran @ 2014-08-23 19:52 UTC (permalink / raw)
To: Fugang Duan; +Cc: davem, netdev
In-Reply-To: <1408612178-13906-2-git-send-email-b38611@freescale.com>
On Thu, Aug 21, 2014 at 05:09:38PM +0800, Fugang Duan wrote:
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
> index 82386b2..cca3617 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -245,12 +245,20 @@ static int fec_ptp_settime(struct ptp_clock_info *ptp,
> u64 ns;
> unsigned long flags;
>
> + mutex_lock(&fep->ptp_clk_mutex);
> + /* Check the ptp clock */
> + if (!fep->ptp_clk_on) {
> + mutex_unlock(&fep->ptp_clk_mutex);
> + return -EINVAL;
> + }
> +
Don't you need the same kind of check in fec_ptp_gettime, too?
> ns = ts->tv_sec * 1000000000ULL;
> ns += ts->tv_nsec;
>
> spin_lock_irqsave(&fep->tmreg_lock, flags);
> timecounter_init(&fep->tc, &fep->cc, ns);
> spin_unlock_irqrestore(&fep->tmreg_lock, flags);
> + mutex_unlock(&fep->ptp_clk_mutex);
> return 0;
> }
>
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next] net: use reciprocal_scale() helper
From: David Miller @ 2014-08-23 19:21 UTC (permalink / raw)
To: dborkman; +Cc: netdev, netfilter-devel, hannes
In-Reply-To: <1408820334-2088-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Sat, 23 Aug 2014 20:58:54 +0200
> Replace open codings of (((u64) <x> * <y>) >> 32) with reciprocal_scale().
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: Header length compution function
From: David Miller @ 2014-08-23 19:19 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: eric.dumazet, amirv, netdev, ogerlitz, yevgenyp, idos
In-Reply-To: <53DA61FE.40406@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Thu, 31 Jul 2014 08:34:22 -0700
> On 07/30/2014 06:39 PM, David Miller wrote:
>> I don't think my proposed patch is a bad trade off. Where we have the
>> __skb_header_pointer() thing that takes preloaded pointers and header
>> length values. It adds only one test which frankly should never
>> trigger and can be moved down into skb_copy_bits() or similar.
>
> This works for me. Once it is in I can see about pushing a patch to add
> some FCoE support and work on moving over igb and ixgbe.
You should be able to do this against net-next now, just FYI.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox