* Re: [PATCH net-next 1/2] ipv4: add forwarding_uses_pmtu knob to protect forward path to use pmtu info
From: Hannes Frederic Sowa @ 2014-01-06 13:29 UTC (permalink / raw)
To: Steffen Klassert; +Cc: netdev, eric.dumazet, davem
In-Reply-To: <20140106131822.GU31491@secunet.com>
On Mon, Jan 06, 2014 at 02:18:22PM +0100, Steffen Klassert wrote:
> On Mon, Jan 06, 2014 at 10:05:21AM +0100, Steffen Klassert wrote:
> > >
> > > IIRC you have a (semi-)automatic test suite to test for (p)mtu problems? Would
> > > these checks cover such a change?
> > >
> >
> > I'm currenlty testing these patches. ipv4 looks good but on
> > ipv6 with 'ping6' the packet size is not reduced according
> > to the pmtu when forward_use_pmtu is set to 0.
> >
> > I'll run the tests again with your updated v3 patches.
> >
>
> The v3 version of your patches don't show any obvious regressions
> on our pmtu test suite.
I am happy to hear that. Thanks again for testing!
^ permalink raw reply
* Re: [net PATCH] netfilter: only warn once on wrong seqadj usage
From: Pablo Neira Ayuso @ 2014-01-06 13:23 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: David S. Miller, netdev, netfilter-devel, Florian Westphal,
Daniel Borkmann, Flavio Leitner
In-Reply-To: <20140104130944.28481.30891.stgit@dragon>
On Sat, Jan 04, 2014 at 02:10:43PM +0100, Jesper Dangaard Brouer wrote:
> Avoid potentially spamming the kernel log with WARN splash messages
> when catching wrong usage of seqadj, by simply using WARN_ONCE.
>
> This is a followup to commit db12cf274353 (netfilter: WARN about
> wrong usage of sequence number adjustments)
>
> Suggested-by: Flavio Leitner <fbl@redhat.com>
> Suggested-by: Daniel Borkmann <dborkman@redhat.com>
> Suggested-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Applied, thanks Jesper.
^ permalink raw reply
* Re: [PATCH v2] net: can: Disable flexcan driver build for big endian CPU on ARM
From: Marc Kleine-Budde @ 2014-01-06 13:23 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <1389014463-14649-1-git-send-email-linux@roeck-us.net>
[-- Attachment #1: Type: text/plain, Size: 810 bytes --]
On 01/06/2014 02:21 PM, Guenter Roeck wrote:
> Building arm:allmodconfig fails with
>
> flexcan.c: In function 'flexcan_read':
> flexcan.c:243:2: error: implicit declaration of function 'in_be32'
> flexcan.c: In function 'flexcan_write':
> flexcan.c:248:2: error: implicit declaration of function 'out_be32'
>
> in_be32 and out_be32 do not (or no longer) exist for ARM targets.
> Disable the build for ARM on big endian CPUs.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Applied to can-next.
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* [PATCH v2] net: can: Disable flexcan driver build for big endian CPU on ARM
From: Guenter Roeck @ 2014-01-06 13:21 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde; +Cc: linux-can, netdev, Guenter Roeck
Building arm:allmodconfig fails with
flexcan.c: In function 'flexcan_read':
flexcan.c:243:2: error: implicit declaration of function 'in_be32'
flexcan.c: In function 'flexcan_write':
flexcan.c:248:2: error: implicit declaration of function 'out_be32'
in_be32 and out_be32 do not (or no longer) exist for ARM targets.
Disable the build for ARM on big endian CPUs.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Instead of marking the build as broken, disable it for big endian ARM CPUs.
Adjust headline accordingly.
drivers/net/can/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 3c06947..fd0e5de 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -104,7 +104,7 @@ config CAN_JANZ_ICAN3
config CAN_FLEXCAN
tristate "Support for Freescale FLEXCAN based chips"
- depends on ARM || PPC
+ depends on (ARM && CPU_LITTLE_ENDIAN) || PPC
---help---
Say Y here if you want to support for Freescale FlexCAN.
--
1.7.9.7
^ permalink raw reply related
* Re: [PATCH net-next 1/2] ipv4: add forwarding_uses_pmtu knob to protect forward path to use pmtu info
From: Steffen Klassert @ 2014-01-06 13:18 UTC (permalink / raw)
To: Hannes Frederic Sowa, netdev, eric.dumazet, davem
In-Reply-To: <20140106090521.GQ31491@secunet.com>
On Mon, Jan 06, 2014 at 10:05:21AM +0100, Steffen Klassert wrote:
> >
> > IIRC you have a (semi-)automatic test suite to test for (p)mtu problems? Would
> > these checks cover such a change?
> >
>
> I'm currenlty testing these patches. ipv4 looks good but on
> ipv6 with 'ping6' the packet size is not reduced according
> to the pmtu when forward_use_pmtu is set to 0.
>
> I'll run the tests again with your updated v3 patches.
>
The v3 version of your patches don't show any obvious regressions
on our pmtu test suite.
^ permalink raw reply
* Re: [PATCH 01/12] netfilter: avoid get_random_bytes calls
From: Florian Westphal @ 2014-01-06 13:07 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel, davem, netdev
In-Reply-To: <20140106130616.GA8187@localhost>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jan 06, 2014 at 02:04:51PM +0100, Florian Westphal wrote:
> > Pablo/David, if you think this needs to be fixed RIGHT NOW then please
> > just issue a revert for a42b99a6e329654d376b330de057eff87686d890.
>
> I just did it, thanks Florian.
Ok. Can you resurrect the nfnetlink_log change?
That one is safe :)
^ permalink raw reply
* Re: [PATCH 01/12] netfilter: avoid get_random_bytes calls
From: Pablo Neira Ayuso @ 2014-01-06 13:06 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <20140106130451.GF28854@breakpoint.cc>
On Mon, Jan 06, 2014 at 02:04:51PM +0100, Florian Westphal wrote:
> Pablo/David, if you think this needs to be fixed RIGHT NOW then please
> just issue a revert for a42b99a6e329654d376b330de057eff87686d890.
I just did it, thanks Florian.
^ permalink raw reply
* Re: [Xen-devel] [PATCH net] xen-netback: fix guest-receive-side array sizes
From: Stefano Stabellini @ 2014-01-06 13:04 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Paul Durrant, netdev, xen-devel, Ian Campbell, David Vrabel,
Konrad Rzeszutek Wilk, Wei Liu, davem
In-Reply-To: <alpine.DEB.2.02.1401061257360.8667@kaball.uk.xensource.com>
On Mon, 6 Jan 2014, Stefano Stabellini wrote:
> On Mon, 23 Dec 2013, Paul Durrant wrote:
> > The sizes chosen for the metadata and grant_copy_op arrays on the guest
> > receive size are wrong;
> >
> > - The meta array is needlessly twice the ring size, when we only ever
> > consume a single array element per RX ring slot
> > - The grant_copy_op array is way too small. It's sized based on a bogus
> > assumption: that at most two copy ops will be used per ring slot. This
> > may have been true at some point in the past but it's clear from looking
> > at start_new_rx_buffer() that a new ring slot is only consumed if a frag
> > would overflow the current slot (plus some other conditions) so the actual
> > limit is MAX_SKB_FRAGS grant_copy_ops per ring slot.
> >
> > This patch fixes those two sizing issues and, because grant_copy_ops grows
> > so much, it pulls it out into a separate chunk of vmalloc()ed memory.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
>
> Unfortunately this patch (now in 3.13-rc7) breaks the ARM build:
>
> CC drivers/net/xen-netback/interface.o
> drivers/net/xen-netback/interface.c: In function 'xenvif_alloc':
> drivers/net/xen-netback/interface.c:311:2: error: implicit declaration of function 'vmalloc' [-Werror=implicit-function-declaration]
> vif->grant_copy_op = vmalloc(sizeof(struct gnttab_copy) *
> ^
> drivers/net/xen-netback/interface.c:311:21: warning: assignment makes pointer from integer without a cast [enabled by default]
> vif->grant_copy_op = vmalloc(sizeof(struct gnttab_copy) *
> ^
> drivers/net/xen-netback/interface.c: In function 'xenvif_free':
> drivers/net/xen-netback/interface.c:499:2: error: implicit declaration of function 'vfree' [-Werror=implicit-function-declaration]
> vfree(vif->grant_copy_op);
> ^
> cc1: some warnings being treated as errors
> make[3]: *** [drivers/net/xen-netback/interface.o] Error 1
> make[2]: *** [drivers/net/xen-netback] Error 2
> make[1]: *** [drivers/net] Error 2
> make: *** [drivers] Error 2
>
> I suggest we fix it (probably by reverting it) ASAP otherwise we risk
> break the release.
Actually I realized that the issue has already been fixed, thanks!
http://marc.info/?l=linux-kernel&m=138897212904706&w=2
^ permalink raw reply
* Re: [PATCH 01/12] netfilter: avoid get_random_bytes calls
From: Florian Westphal @ 2014-01-06 13:04 UTC (permalink / raw)
To: Florian Westphal, Pablo Neira Ayuso, netfilter-devel, davem,
netdev
In-Reply-To: <20140106124340.GC4611@order.stressinduktion.org>
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> On Mon, Jan 06, 2014 at 12:54:36PM +0100, Florian Westphal wrote:
> > Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> > Can you elaborate? If entropy estimate is really really low
> > (because we're booting up), why would get_random_bytes() be a better
> > choice [ i understand net_get_random_once() is for delaying
> > the actual random_bytes call until a later point in time where we've
> > hopefully collected more entropy ]
[..]
> On some of my small virtual machines (amd64) I even see this message while
> login on the console (small iptables set also loaded before). In the mean
> time prandom_u32() is still seeded with maybe 3 bits (I once measured it)
> at the beginning and won't get a refresh until the nonblocking pool is
> fully initialized.
I see. In this case it indeed could be a problem; I was doing this
change with the assumption that prandom is useable at ->checkenty time.
> > I specifically did not use net_get_random_once once because checkentry is
> > not a hotpath.
> >
> > I don't see why get_random_bytes use increases the security margin, especially
> > considering none of these hashes have periodic run-time rehashing?
> >
> > But sure, if you think this change is a problem, Pablo can just revert it.
>
> I don't know if it is a real problem. Most of the time the initial seed
> should be enough, but I guess get_random_bytes would still be a more
> defensive choice. I would have used it. ;)
Alright. Given that this went into -next, I think we have a few weeks to
investigate.
I will check if the specific hash uses are problematic in their own right
(due to lack of reseed) or if they are weakened by this change only.
I'll follow up on this.
Pablo/David, if you think this needs to be fixed RIGHT NOW then please
just issue a revert for a42b99a6e329654d376b330de057eff87686d890.
Thanks!
^ permalink raw reply
* Re: [Xen-devel] [PATCH net] xen-netback: fix guest-receive-side array sizes
From: Stefano Stabellini @ 2014-01-06 12:59 UTC (permalink / raw)
To: Paul Durrant
Cc: netdev, xen-devel, Ian Campbell, David Vrabel,
Konrad Rzeszutek Wilk, Wei Liu, davem
In-Reply-To: <1387790838-8852-1-git-send-email-paul.durrant@citrix.com>
On Mon, 23 Dec 2013, Paul Durrant wrote:
> The sizes chosen for the metadata and grant_copy_op arrays on the guest
> receive size are wrong;
>
> - The meta array is needlessly twice the ring size, when we only ever
> consume a single array element per RX ring slot
> - The grant_copy_op array is way too small. It's sized based on a bogus
> assumption: that at most two copy ops will be used per ring slot. This
> may have been true at some point in the past but it's clear from looking
> at start_new_rx_buffer() that a new ring slot is only consumed if a frag
> would overflow the current slot (plus some other conditions) so the actual
> limit is MAX_SKB_FRAGS grant_copy_ops per ring slot.
>
> This patch fixes those two sizing issues and, because grant_copy_ops grows
> so much, it pulls it out into a separate chunk of vmalloc()ed memory.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
Unfortunately this patch (now in 3.13-rc7) breaks the ARM build:
CC drivers/net/xen-netback/interface.o
drivers/net/xen-netback/interface.c: In function 'xenvif_alloc':
drivers/net/xen-netback/interface.c:311:2: error: implicit declaration of function 'vmalloc' [-Werror=implicit-function-declaration]
vif->grant_copy_op = vmalloc(sizeof(struct gnttab_copy) *
^
drivers/net/xen-netback/interface.c:311:21: warning: assignment makes pointer from integer without a cast [enabled by default]
vif->grant_copy_op = vmalloc(sizeof(struct gnttab_copy) *
^
drivers/net/xen-netback/interface.c: In function 'xenvif_free':
drivers/net/xen-netback/interface.c:499:2: error: implicit declaration of function 'vfree' [-Werror=implicit-function-declaration]
vfree(vif->grant_copy_op);
^
cc1: some warnings being treated as errors
make[3]: *** [drivers/net/xen-netback/interface.o] Error 1
make[2]: *** [drivers/net/xen-netback] Error 2
make[1]: *** [drivers/net] Error 2
make: *** [drivers] Error 2
I suggest we fix it (probably by reverting it) ASAP otherwise we risk
break the release.
> This was originally submitted for discussion on xen-devel. Wei acked it
> there, which is why this carbon-copy submission to netdev already carries
> his ack.
>
> drivers/net/xen-netback/common.h | 19 +++++++++++++------
> drivers/net/xen-netback/interface.c | 10 ++++++++++
> drivers/net/xen-netback/netback.c | 2 +-
> 3 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 08ae01b..c47794b 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -101,6 +101,13 @@ struct xenvif_rx_meta {
>
> #define MAX_PENDING_REQS 256
>
> +/* It's possible for an skb to have a maximal number of frags
> + * but still be less than MAX_BUFFER_OFFSET in size. Thus the
> + * worst-case number of copy operations is MAX_SKB_FRAGS per
> + * ring slot.
> + */
> +#define MAX_GRANT_COPY_OPS (MAX_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
> +
> struct xenvif {
> /* Unique identifier for this interface. */
> domid_t domid;
> @@ -143,13 +150,13 @@ struct xenvif {
> */
> RING_IDX rx_req_cons_peek;
>
> - /* Given MAX_BUFFER_OFFSET of 4096 the worst case is that each
> - * head/fragment page uses 2 copy operations because it
> - * straddles two buffers in the frontend.
> - */
> - struct gnttab_copy grant_copy_op[2*XEN_NETIF_RX_RING_SIZE];
> - struct xenvif_rx_meta meta[2*XEN_NETIF_RX_RING_SIZE];
> + /* This array is allocated seperately as it is large */
> + struct gnttab_copy *grant_copy_op;
>
> + /* We create one meta structure per ring request we consume, so
> + * the maximum number is the same as the ring size.
> + */
> + struct xenvif_rx_meta meta[XEN_NETIF_RX_RING_SIZE];
>
> u8 fe_dev_addr[6];
>
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 870f1fa..34ca4e5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -307,6 +307,15 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
> SET_NETDEV_DEV(dev, parent);
>
> vif = netdev_priv(dev);
> +
> + vif->grant_copy_op = vmalloc(sizeof(struct gnttab_copy) *
> + MAX_GRANT_COPY_OPS);
> + if (vif->grant_copy_op == NULL) {
> + pr_warn("Could not allocate grant copy space for %s\n", name);
> + free_netdev(dev);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> vif->domid = domid;
> vif->handle = handle;
> vif->can_sg = 1;
> @@ -487,6 +496,7 @@ void xenvif_free(struct xenvif *vif)
>
> unregister_netdev(vif->dev);
>
> + vfree(vif->grant_copy_op);
> free_netdev(vif->dev);
>
> module_put(THIS_MODULE);
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 7b4fd93..7842555 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -608,7 +608,7 @@ void xenvif_rx_action(struct xenvif *vif)
> if (!npo.copy_prod)
> return;
>
> - BUG_ON(npo.copy_prod > ARRAY_SIZE(vif->grant_copy_op));
> + BUG_ON(npo.copy_prod > MAX_GRANT_COPY_OPS);
> gnttab_batch_copy(vif->grant_copy_op, npo.copy_prod);
>
> while ((skb = __skb_dequeue(&rxq)) != NULL) {
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply
* Re: [PATCH 01/12] netfilter: avoid get_random_bytes calls
From: Pablo Neira Ayuso @ 2014-01-06 12:58 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Florian Westphal, netfilter-devel, davem, netdev
In-Reply-To: <20140106124340.GC4611@order.stressinduktion.org>
On Mon, Jan 06, 2014 at 01:43:40PM +0100, Hannes Frederic Sowa wrote:
[...]
> > > Hmm, some of them look like good candidates for net_get_random_once. I don't
> > > see such a problem with draining entropy pool, especially as they don't run
> > > that early and they don't request so many random bits.
> >
> > I specifically did not use net_get_random_once once because checkentry is
> > not a hotpath.
> >
> > I don't see why get_random_bytes use increases the security margin, especially
> > considering none of these hashes have periodic run-time rehashing?
> >
> > But sure, if you think this change is a problem, Pablo can just revert it.
>
> I don't know if it is a real problem. Most of the time the initial seed
> should be enough, but I guess get_random_bytes would still be a more
> defensive choice. I would have used it. ;)
OK, I have reverted this patch, thanks.
^ permalink raw reply
* Re: [PATCH 01/12] netfilter: avoid get_random_bytes calls
From: Hannes Frederic Sowa @ 2014-01-06 12:43 UTC (permalink / raw)
To: Florian Westphal; +Cc: Pablo Neira Ayuso, netfilter-devel, davem, netdev
In-Reply-To: <20140106115436.GE28854@breakpoint.cc>
Hello!
On Mon, Jan 06, 2014 at 12:54:36PM +0100, Florian Westphal wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> > On Mon, Jan 06, 2014 at 12:12:55AM +0100, Pablo Neira Ayuso wrote:
> > > From: Florian Westphal <fw@strlen.de>
> > >
> > > All these users need an initial seed value for jhash, prandom is
> > > perfectly fine. This avoids draining the entropy pool where
> > > its not strictly required.
> >
> > Secrets protecting hash tables should be rather strong.
>
> Yes, which is why e.g. conntrack hash is not converted.
>
> > prandom_u32() has two seeding points at boot-up. One is at late_initcall.
>
> Yes. None of these locations are executed via initcalls, they are all
> in _checkentry (i.e., run when userspace iptables inserts a rule using
> the target/match), except hashlimit where its delayed until the first
> address is stored (so its even later).
>
> > Thanks to parallel boot-up this gets executed fairly early. The other one is
> > when the RNG nonblocking pool is fully initialized. Only after this point we
> > can assume prandom_u32() returns truely random values. In between, only
> > get_random_bytes or net_get_random_once are safe for use.
>
> Can you elaborate? If entropy estimate is really really low
> (because we're booting up), why would get_random_bytes() be a better
> choice [ i understand net_get_random_once() is for delaying
> the actual random_bytes call until a later point in time where we've
> hopefully collected more entropy ]
I hope, I answer that below.
> > To get the impression when prandom_u32 gets truely seeded, watch out
> > for the message "random: nonblocking pool is initialized" in dmesg. ;)
>
> It happens very very early on my machine, even before / is remounted
> rw. I would be more interested in what happens on small embedded
> boxes...
On some of my small virtual machines (amd64) I even see this message while
login on the console (small iptables set also loaded before). In the mean
time prandom_u32() is still seeded with maybe 3 bits (I once measured it)
at the beginning and won't get a refresh until the nonblocking pool is
fully initialized. prandom_u32 will just iterate over its seed until it
is renewed whereas get_random_bytes does try to stretch (with help of the
twisted GFSR and SHA-1) the available entropy in case the nonblocking_pool
is limited, thus it is more probable to get better random results.
E.g. on an amd64 athlon x2 with two VMs:
[Mon Jan 6 13:35:40 2014] Initializing cgroup subsys cpuset
...
[Mon Jan 6 13:36:21 2014] random: nonblocking pool is initialized
I normally get the message while typing in the password on the prompt of the
serial console.
Single integers are not so much of a problem. E.g. one problem in
wireless code was, where get_random_bytes was called in a loop to fill
a structure, that did hurt: f7d8ad81ca8c44 ("mac80211: minstrels: spare
numerous useless calls to get_random_bytes").
> > Hmm, some of them look like good candidates for net_get_random_once. I don't
> > see such a problem with draining entropy pool, especially as they don't run
> > that early and they don't request so many random bits.
>
> I specifically did not use net_get_random_once once because checkentry is
> not a hotpath.
>
> I don't see why get_random_bytes use increases the security margin, especially
> considering none of these hashes have periodic run-time rehashing?
>
> But sure, if you think this change is a problem, Pablo can just revert it.
I don't know if it is a real problem. Most of the time the initial seed
should be enough, but I guess get_random_bytes would still be a more
defensive choice. I would have used it. ;)
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net 2/2] net: core: explicitly select a txq before doing l2 forwarding
From: Neil Horman @ 2014-01-06 12:42 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel, mst, John Fastabend, e1000-devel
In-Reply-To: <1388978467-2075-2-git-send-email-jasowang@redhat.com>
On Mon, Jan 06, 2014 at 11:21:07AM +0800, Jason Wang wrote:
> Currently, the tx queue were selected implicitly in ndo_dfwd_start_xmit(). The
> will cause several issues:
>
> - NETIF_F_LLTX was forced for macvlan device in this case which lead extra lock
> contention.
> - dev_hard_start_xmit() was called with NULL txq which bypasses the net device
> watchdog
> - dev_hard_start_xmit() does not check txq everywhere which will lead a crash
> when tso is disabled for lower device.
>
> Fix this by explicitly introducing a select queue method just for l2 forwarding
> offload (ndo_dfwd_select_queue), and introducing dfwd_direct_xmit() to do the
> queue selecting and transmitting for l2 forwarding.
>
> With this fixes, NETIF_F_LLTX could be preserved for macvlan and there's no need
> to check txq against NULL in dev_hard_start_xmit().
>
> In the future, it was also required for macvtap l2 forwarding support since it
> provides a necessary synchronization method.
>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: e1000-devel@lists.sourceforge.net
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Instead of creating another operation here to do special queue selection, why
not just have ndo_dfwd_start_xmit include a pointer to a pointer in its argument
list, so it can pass the txq it used back to the caller (dev_hard_start_xmit)?
ndo_dfwd_start_xmit already knows which queue set to pick from (since their
reserved for the device doing the transmitting). It seems more clear to me than
creating a new netdevice operation.
As for the crash issue, I'm not sure what you mean. Where in
dev_hard_start_xmit would we need to check txq that we're not currently, and
what crash results?
Also, can you elaborate on what you mean by additional lock contention? What
contention do you see that goes above and beyond the normal locking required by
txq access? I suppose its extra locking above and beyond in the macvtap case,
where you would otherwise never hit hardware, but that not the only use case,
and I think the solution there is likely to add some code in the macvlan feature
set handler so that NETIF_F_LLTX is cleared if you disable the hardware
forwarding acceleration via ethtool.
Regards
Neil
>
^ permalink raw reply
* [net-next 15/15] i40e: Do not allow AQ calls from ndo-ops
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Anjali Singhai Jain, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Anjali Singhai Jain <anjali.singhai@intel.com>
If the device is not in a working state avoid making admin
queue (AQ) calls that rely on a working AQ.
Change-Id: Ifbba6d257b3a5b51bfe92938c04088c0baa21433
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 30 +++++++++++++----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index fd24ee4..483126c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1215,6 +1215,10 @@ static int i40e_set_mac(struct net_device *netdev, void *p)
if (ether_addr_equal(netdev->dev_addr, addr->sa_data))
return 0;
+ if (test_bit(__I40E_DOWN, &vsi->back->state) ||
+ test_bit(__I40E_RESET_RECOVERY_PENDING, &vsi->back->state))
+ return -EADDRNOTAVAIL;
+
if (vsi->type == I40E_VSI_MAIN) {
i40e_status ret;
ret = i40e_aq_mac_address_write(&vsi->back->hw,
@@ -1779,7 +1783,6 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid)
{
struct i40e_mac_filter *f, *add_f;
bool is_netdev, is_vf;
- int ret;
is_vf = (vsi->type == I40E_VSI_SRIOV);
is_netdev = !!(vsi->netdev);
@@ -1805,13 +1808,6 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid)
}
}
- ret = i40e_sync_vsi_filters(vsi);
- if (ret) {
- dev_info(&vsi->back->pdev->dev,
- "Could not sync filters for vid %d\n", vid);
- return ret;
- }
-
/* Now if we add a vlan tag, make sure to check if it is the first
* tag (i.e. a "tag" -1 does exist) and if so replace the -1 "tag"
* with 0, so we now accept untagged and specified tagged traffic
@@ -1848,10 +1844,13 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid)
}
}
}
- ret = i40e_sync_vsi_filters(vsi);
}
- return ret;
+ if (test_bit(__I40E_DOWN, &vsi->back->state) ||
+ test_bit(__I40E_RESET_RECOVERY_PENDING, &vsi->back->state))
+ return 0;
+
+ return i40e_sync_vsi_filters(vsi);
}
/**
@@ -1867,7 +1866,6 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
struct i40e_mac_filter *f, *add_f;
bool is_vf, is_netdev;
int filter_count = 0;
- int ret;
is_vf = (vsi->type == I40E_VSI_SRIOV);
is_netdev = !!(netdev);
@@ -1878,12 +1876,6 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
list_for_each_entry(f, &vsi->mac_filter_list, list)
i40e_del_filter(vsi, f->macaddr, vid, is_vf, is_netdev);
- ret = i40e_sync_vsi_filters(vsi);
- if (ret) {
- dev_info(&vsi->back->pdev->dev, "Could not sync filters\n");
- return ret;
- }
-
/* go through all the filters for this VSI and if there is only
* vid == 0 it means there are no other filters, so vid 0 must
* be replaced with -1. This signifies that we should from now
@@ -1926,6 +1918,10 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
}
}
+ if (test_bit(__I40E_DOWN, &vsi->back->state) ||
+ test_bit(__I40E_RESET_RECOVERY_PENDING, &vsi->back->state))
+ return 0;
+
return i40e_sync_vsi_filters(vsi);
}
--
1.8.3.1
^ permalink raw reply related
* [net-next 14/15] i40e: check asq alive before notify
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Anjali Singhai Jain, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Anjali Singhai Jain <anjali.singhai@intel.com>
Driver needs to make sure the send queue is alive before
trying to use it.
Chagne-Id: I9bd1f6159c45c98e63f562e3a8dfb57edfe50e13
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c97fc0c..fd24ee4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -4776,7 +4776,8 @@ static int i40e_prep_for_reset(struct i40e_pf *pf)
dev_info(&pf->pdev->dev, "Tearing down internal switch for reset\n");
- i40e_vc_notify_reset(pf);
+ if (i40e_check_asq_alive(hw))
+ i40e_vc_notify_reset(pf);
/* quiesce the VSIs and their queues that are not already DOWN */
i40e_pf_quiesce_all_vsi(pf);
--
1.8.3.1
^ permalink raw reply related
* [net-next 12/15] i40e: Hide the Port VLAN VLAN ID
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem; +Cc: Greg Rose, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Greg Rose <gregory.v.rose@intel.com>
The VF VSI Port VLAN settings still allow the user to view VLAN tag in
the descriptor. Fix the settings to hide the VLAN ID from the VF. The
VF is not supposed to be aware it is on a VLAN in the Port VLAN
scenario.
Change-Id: I976f2bacb455dbb750f8c53a781c689f02cb8907
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 40484e0..9c16aa8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2018,7 +2018,7 @@ int i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
vsi->info.pvid = cpu_to_le16(vid);
vsi->info.port_vlan_flags = I40E_AQ_VSI_PVLAN_MODE_TAGGED |
I40E_AQ_VSI_PVLAN_INSERT_PVID |
- I40E_AQ_VSI_PVLAN_EMOD_STR_BOTH;
+ I40E_AQ_VSI_PVLAN_EMOD_STR;
ctxt.seid = vsi->seid;
memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
--
1.8.3.1
^ permalink raw reply related
* [net-next 13/15] i40e: Admin queue shutdown fixes
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Anjali Singhai Jain, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Anjali Singhai Jain <anjali.singhai@intel.com>
Always call the AQ call to shutdown the queue in the shutdown path.
Check ASQ is alive before issuing the AQ command since we might be
resetting to recover from a bad state in which case we should not
issue the AQ command.
Use the register variable for length so it can be used by PF, VF
and GL AQ commands.
Change-Id: Ic3d305687ea3f1a6afa84e864b7a27bd38a9af32
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 3 ++
drivers/net/ethernet/intel/i40e/i40e_common.c | 63 ++++++++++++++----------
drivers/net/ethernet/intel/i40e/i40e_main.c | 1 -
drivers/net/ethernet/intel/i40e/i40e_prototype.h | 5 +-
4 files changed, 44 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index d04ad3b..f75b573 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -609,6 +609,9 @@ i40e_status i40e_shutdown_adminq(struct i40e_hw *hw)
{
i40e_status ret_code = 0;
+ if (i40e_check_asq_alive(hw))
+ i40e_aq_queue_shutdown(hw, true);
+
i40e_shutdown_asq(hw);
i40e_shutdown_arq(hw);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 161d6ff..7cd59ce 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -126,6 +126,44 @@ void i40e_debug_aq(struct i40e_hw *hw, enum i40e_debug_mask mask, void *desc,
}
/**
+ * i40e_check_asq_alive
+ * @hw: pointer to the hw struct
+ *
+ * Returns true if Queue is enabled else false.
+ **/
+bool i40e_check_asq_alive(struct i40e_hw *hw)
+{
+ return !!(rd32(hw, hw->aq.asq.len) & I40E_PF_ATQLEN_ATQENABLE_MASK);
+}
+
+/**
+ * i40e_aq_queue_shutdown
+ * @hw: pointer to the hw struct
+ * @unloading: is the driver unloading itself
+ *
+ * Tell the Firmware that we're shutting down the AdminQ and whether
+ * or not the driver is unloading as well.
+ **/
+i40e_status i40e_aq_queue_shutdown(struct i40e_hw *hw,
+ bool unloading)
+{
+ struct i40e_aq_desc desc;
+ struct i40e_aqc_queue_shutdown *cmd =
+ (struct i40e_aqc_queue_shutdown *)&desc.params.raw;
+ i40e_status status;
+
+ i40e_fill_default_direct_cmd_desc(&desc,
+ i40e_aqc_opc_queue_shutdown);
+
+ if (unloading)
+ cmd->driver_unloading = cpu_to_le32(I40E_AQ_DRIVER_UNLOADING);
+ status = i40e_asq_send_command(hw, &desc, NULL, 0, NULL);
+
+ return status;
+}
+
+
+/**
* i40e_init_shared_code - Initialize the shared code
* @hw: pointer to hardware structure
*
@@ -478,31 +516,6 @@ void i40e_led_set(struct i40e_hw *hw, u32 mode, bool blink)
}
/* Admin command wrappers */
-/**
- * i40e_aq_queue_shutdown
- * @hw: pointer to the hw struct
- * @unloading: is the driver unloading itself
- *
- * Tell the Firmware that we're shutting down the AdminQ and whether
- * or not the driver is unloading as well.
- **/
-i40e_status i40e_aq_queue_shutdown(struct i40e_hw *hw,
- bool unloading)
-{
- struct i40e_aq_desc desc;
- struct i40e_aqc_queue_shutdown *cmd =
- (struct i40e_aqc_queue_shutdown *)&desc.params.raw;
- i40e_status status;
-
- i40e_fill_default_direct_cmd_desc(&desc,
- i40e_aqc_opc_queue_shutdown);
-
- if (unloading)
- cmd->driver_unloading = cpu_to_le32(I40E_AQ_DRIVER_UNLOADING);
- status = i40e_asq_send_command(hw, &desc, NULL, 0, NULL);
-
- return status;
-}
/**
* i40e_aq_set_link_restart_an
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 9c16aa8..c97fc0c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -7854,7 +7854,6 @@ static void i40e_remove(struct pci_dev *pdev)
"Failed to destroy the HMC resources: %d\n", ret_code);
/* shutdown the adminq */
- i40e_aq_queue_shutdown(&pf->hw, true);
ret_code = i40e_shutdown_adminq(&pf->hw);
if (ret_code)
dev_warn(&pdev->dev,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
index 800f3c8..e05d303 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
@@ -59,6 +59,9 @@ void i40e_debug_aq(struct i40e_hw *hw,
void *buffer);
void i40e_idle_aq(struct i40e_hw *hw);
+bool i40e_check_asq_alive(struct i40e_hw *hw);
+i40e_status i40e_aq_queue_shutdown(struct i40e_hw *hw,
+ bool unloading);
u32 i40e_led_get(struct i40e_hw *hw);
void i40e_led_set(struct i40e_hw *hw, u32 mode, bool blink);
@@ -69,8 +72,6 @@ i40e_status i40e_aq_get_firmware_version(struct i40e_hw *hw,
u16 *fw_major_version, u16 *fw_minor_version,
u16 *api_major_version, u16 *api_minor_version,
struct i40e_asq_cmd_details *cmd_details);
-i40e_status i40e_aq_queue_shutdown(struct i40e_hw *hw,
- bool unloading);
i40e_status i40e_aq_set_phy_reset(struct i40e_hw *hw,
struct i40e_asq_cmd_details *cmd_details);
i40e_status i40e_aq_set_default_vsi(struct i40e_hw *hw, u16 vsi_id,
--
1.8.3.1
^ permalink raw reply related
* [net-next 11/15] i40e: use correct struct for get and update vsi params
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Shannon Nelson, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Shannon Nelson <shannon.nelson@intel.com>
The get_vsi_params and update_vsi_params functions were using a
different command struct that just happened to have an seid element in
the right place and so worked correctly anyway. This patch fixes the
functions to use the right data struct.
There is no actual logic change.
Change-Id: I513b5e1dc293dfd5b2ba4fa443cbdbfa608d9d19
Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_common.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index e5694b9..161d6ff 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -748,8 +748,8 @@ i40e_status i40e_aq_get_vsi_params(struct i40e_hw *hw,
struct i40e_asq_cmd_details *cmd_details)
{
struct i40e_aq_desc desc;
- struct i40e_aqc_switch_seid *cmd =
- (struct i40e_aqc_switch_seid *)&desc.params.raw;
+ struct i40e_aqc_add_get_update_vsi *cmd =
+ (struct i40e_aqc_add_get_update_vsi *)&desc.params.raw;
struct i40e_aqc_add_get_update_vsi_completion *resp =
(struct i40e_aqc_add_get_update_vsi_completion *)
&desc.params.raw;
@@ -758,7 +758,7 @@ i40e_status i40e_aq_get_vsi_params(struct i40e_hw *hw,
i40e_fill_default_direct_cmd_desc(&desc,
i40e_aqc_opc_get_vsi_parameters);
- cmd->seid = cpu_to_le16(vsi_ctx->seid);
+ cmd->uplink_seid = cpu_to_le16(vsi_ctx->seid);
desc.flags |= cpu_to_le16((u16)I40E_AQ_FLAG_BUF);
if (sizeof(vsi_ctx->info) > I40E_AQ_LARGE_BUF)
@@ -792,13 +792,13 @@ i40e_status i40e_aq_update_vsi_params(struct i40e_hw *hw,
struct i40e_asq_cmd_details *cmd_details)
{
struct i40e_aq_desc desc;
- struct i40e_aqc_switch_seid *cmd =
- (struct i40e_aqc_switch_seid *)&desc.params.raw;
+ struct i40e_aqc_add_get_update_vsi *cmd =
+ (struct i40e_aqc_add_get_update_vsi *)&desc.params.raw;
i40e_status status;
i40e_fill_default_direct_cmd_desc(&desc,
i40e_aqc_opc_update_vsi_parameters);
- cmd->seid = cpu_to_le16(vsi_ctx->seid);
+ cmd->uplink_seid = cpu_to_le16(vsi_ctx->seid);
desc.flags |= cpu_to_le16((u16)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
if (sizeof(vsi_ctx->info) > I40E_AQ_LARGE_BUF)
--
1.8.3.1
^ permalink raw reply related
* [net-next 10/15] i40e: Fix VF driver MAC address configuration
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem; +Cc: Greg Rose, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Greg Rose <gregory.v.rose@intel.com>
Fix a problem where the 'ip link show' command would display stale
link address information after the link address was set via the 'ip
link set' command. In addition, fix problem with the user being
allowed to overwrite the administratively set VF MAC address.
Change-Id: I669ed14e55f2b633ef7b456b713632b08468671c
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 60 +++++++++++++++++-----
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h | 1 +
2 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index a983858..55ec2db 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1331,6 +1331,37 @@ error_param:
}
/**
+ * i40e_check_vf_permission
+ * @vf: pointer to the vf info
+ * @macaddr: pointer to the MAC Address being checked
+ *
+ * Check if the VF has permission to add or delete unicast MAC address
+ * filters and return error code -EPERM if not. Then check if the
+ * address filter requested is broadcast or zero and if so return
+ * an invalid MAC address error code.
+ **/
+static inline int i40e_check_vf_permission(struct i40e_vf *vf, u8 *macaddr)
+{
+ struct i40e_pf *pf = vf->pf;
+ int ret = 0;
+
+ if (is_broadcast_ether_addr(macaddr) ||
+ is_zero_ether_addr(macaddr)) {
+ dev_err(&pf->pdev->dev, "invalid VF MAC addr %pM\n", macaddr);
+ ret = I40E_ERR_INVALID_MAC_ADDR;
+ } else if (vf->pf_set_mac && !is_multicast_ether_addr(macaddr)) {
+ /* If the host VMM administrator has set the VF MAC address
+ * administratively via the ndo_set_vf_mac command then deny
+ * permission to the VF to add or delete unicast MAC addresses.
+ */
+ dev_err(&pf->pdev->dev,
+ "VF attempting to override administratively set MAC address\nPlease reload the VF driver to resume normal operation\n");
+ ret = -EPERM;
+ }
+ return ret;
+}
+
+/**
* i40e_vc_add_mac_addr_msg
* @vf: pointer to the vf info
* @msg: pointer to the msg buffer
@@ -1345,24 +1376,20 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen)
struct i40e_pf *pf = vf->pf;
struct i40e_vsi *vsi = NULL;
u16 vsi_id = al->vsi_id;
- i40e_status aq_ret = 0;
+ i40e_status ret = 0;
int i;
if (!test_bit(I40E_VF_STAT_ACTIVE, &vf->vf_states) ||
!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps) ||
!i40e_vc_isvalid_vsi_id(vf, vsi_id)) {
- aq_ret = I40E_ERR_PARAM;
+ ret = I40E_ERR_PARAM;
goto error_param;
}
for (i = 0; i < al->num_elements; i++) {
- if (is_broadcast_ether_addr(al->list[i].addr) ||
- is_zero_ether_addr(al->list[i].addr)) {
- dev_err(&pf->pdev->dev, "invalid VF MAC addr %pMAC\n",
- al->list[i].addr);
- aq_ret = I40E_ERR_INVALID_MAC_ADDR;
+ ret = i40e_check_vf_permission(vf, al->list[i].addr);
+ if (ret)
goto error_param;
- }
}
vsi = pf->vsi[vsi_id];
@@ -1383,7 +1410,7 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen)
if (!f) {
dev_err(&pf->pdev->dev,
"Unable to add VF MAC filter\n");
- aq_ret = I40E_ERR_PARAM;
+ ret = I40E_ERR_PARAM;
goto error_param;
}
}
@@ -1395,7 +1422,7 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen)
error_param:
/* send the response to the vf */
return i40e_vc_send_resp_to_vf(vf, I40E_VIRTCHNL_OP_ADD_ETHER_ADDRESS,
- aq_ret);
+ ret);
}
/**
@@ -1413,15 +1440,21 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen)
struct i40e_pf *pf = vf->pf;
struct i40e_vsi *vsi = NULL;
u16 vsi_id = al->vsi_id;
- i40e_status aq_ret = 0;
+ i40e_status ret = 0;
int i;
if (!test_bit(I40E_VF_STAT_ACTIVE, &vf->vf_states) ||
!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps) ||
!i40e_vc_isvalid_vsi_id(vf, vsi_id)) {
- aq_ret = I40E_ERR_PARAM;
+ ret = I40E_ERR_PARAM;
goto error_param;
}
+
+ for (i = 0; i < al->num_elements; i++) {
+ ret = i40e_check_vf_permission(vf, al->list[i].addr);
+ if (ret)
+ goto error_param;
+ }
vsi = pf->vsi[vsi_id];
/* delete addresses from the list */
@@ -1436,7 +1469,7 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen)
error_param:
/* send the response to the vf */
return i40e_vc_send_resp_to_vf(vf, I40E_VIRTCHNL_OP_DEL_ETHER_ADDRESS,
- aq_ret);
+ ret);
}
/**
@@ -1918,6 +1951,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
goto error_param;
}
memcpy(vf->default_lan_addr.addr, mac, ETH_ALEN);
+ vf->pf_set_mac = true;
dev_info(&pf->pdev->dev, "Reload the VF driver to make this change effective.\n");
ret = 0;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
index bf3384f..8d0f4dd 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
@@ -83,6 +83,7 @@ struct i40e_vf {
struct i40e_virtchnl_ether_addr default_lan_addr;
struct i40e_virtchnl_ether_addr default_fcoe_addr;
u16 port_vlan_id;
+ bool pf_set_mac; /* The VMM admin set the VF MAC address */
/* VSI indices - actual VSI pointers are maintained in the PF structure
* When assigned, these will be non-zero, because VSI 0 is always
--
1.8.3.1
^ permalink raw reply related
* [net-next 09/15] i40e: support VFs on PFs other than 0
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Mitch Williams, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mitch Williams <mitch.a.williams@intel.com>
When communicating with VF devices over the AQ, the FW refers to the
VF by its global VF ID, not local the VF ID with reference to its
parent PF. Since the global and local VF IDs are identical for PF 0,
the code worked correctly on PF 0.
However, we cannot just use global IDs throughout the code as most of
the other references to the VF (VSI setup, register offsets, etc.)
require the local VF ID. Instead, we just add or subtract our base VF
ID when sending and receiving AQ messages.
Change-Id: I92f4332b4876bc68b2f9af9ebf48761f63b6bd97
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index e91f9d7..a983858 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -903,6 +903,7 @@ static int i40e_vc_send_msg_to_vf(struct i40e_vf *vf, u32 v_opcode,
{
struct i40e_pf *pf = vf->pf;
struct i40e_hw *hw = &pf->hw;
+ int true_vf_id = vf->vf_id + hw->func_caps.vf_base_id;
i40e_status aq_ret;
/* single place to detect unsuccessful return values */
@@ -922,8 +923,8 @@ static int i40e_vc_send_msg_to_vf(struct i40e_vf *vf, u32 v_opcode,
vf->num_valid_msgs++;
}
- aq_ret = i40e_aq_send_msg_to_vf(hw, vf->vf_id, v_opcode, v_retval,
- msg, msglen, NULL);
+ aq_ret = i40e_aq_send_msg_to_vf(hw, true_vf_id, v_opcode, v_retval,
+ msg, msglen, NULL);
if (aq_ret) {
dev_err(&pf->pdev->dev,
"Unable to send the message to VF %d aq_err %d\n",
@@ -1664,22 +1665,23 @@ int i40e_vc_process_vf_msg(struct i40e_pf *pf, u16 vf_id, u32 v_opcode,
u32 v_retval, u8 *msg, u16 msglen)
{
struct i40e_hw *hw = &pf->hw;
+ int local_vf_id = vf_id - hw->func_caps.vf_base_id;
struct i40e_vf *vf;
int ret;
pf->vf_aq_requests++;
- if (vf_id >= pf->num_alloc_vfs)
+ if (local_vf_id >= pf->num_alloc_vfs)
return -EINVAL;
- vf = &(pf->vf[vf_id]);
+ vf = &(pf->vf[local_vf_id]);
/* perform basic checks on the msg */
ret = i40e_vc_validate_vf_msg(vf, v_opcode, v_retval, msg, msglen);
if (ret) {
dev_err(&pf->pdev->dev, "Invalid message from vf %d, opcode %d, len %d\n",
- vf_id, v_opcode, msglen);
+ local_vf_id, v_opcode, msglen);
return ret;
}
- wr32(hw, I40E_VFGEN_RSTAT1(vf_id), I40E_VFR_VFACTIVE);
+ wr32(hw, I40E_VFGEN_RSTAT1(local_vf_id), I40E_VFR_VFACTIVE);
switch (v_opcode) {
case I40E_VIRTCHNL_OP_VERSION:
ret = i40e_vc_get_version_msg(vf);
@@ -1723,8 +1725,8 @@ int i40e_vc_process_vf_msg(struct i40e_pf *pf, u16 vf_id, u32 v_opcode,
break;
case I40E_VIRTCHNL_OP_UNKNOWN:
default:
- dev_err(&pf->pdev->dev,
- "Unsupported opcode %d from vf %d\n", v_opcode, vf_id);
+ dev_err(&pf->pdev->dev, "Unsupported opcode %d from vf %d\n",
+ v_opcode, local_vf_id);
ret = i40e_vc_send_resp_to_vf(vf, v_opcode,
I40E_ERR_NOT_IMPLEMENTED);
break;
--
1.8.3.1
^ permalink raw reply related
* [net-next 07/15] i40e: don't allocate zero size
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Mitch Williams, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mitch Williams <mitch.a.williams@intel.com>
Shockingly, the compiler didn't flag this uninitialized variable. This
fixes a potential memory corruption condition where ARQ messages are
written to random memory locations.
Change-Id: Iac82f4562d2bf3f42df3f3b2163d9cbed2160135
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c0d7fe6..40484e0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -4492,6 +4492,7 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
if (!test_bit(__I40E_ADMINQ_EVENT_PENDING, &pf->state))
return;
+ event.msg_size = I40E_MAX_AQ_BUF_SIZE;
event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
if (!event.msg_buf)
return;
--
1.8.3.1
^ permalink raw reply related
* [net-next 08/15] i40e: acknowledge VFLR when disabling SR-IOV
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Mitch Williams, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mitch Williams <mitch.a.williams@intel.com>
When SR-IOV is disabled, the (now nonexistent) virtual function
devices undergo a VFLR event. We don't need to handle this event
because the VFs are gone, but we do need to tell the HW that they are
complete. This fixes an issue with a phantom VFLR and broken VFs when
SR-IOV is re-enabled.
Change-Id: I7580b49ded0158172a85b14661ec212af77000c8
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index f92404c..e91f9d7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -726,7 +726,9 @@ static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
**/
void i40e_free_vfs(struct i40e_pf *pf)
{
- int i, tmp;
+ struct i40e_hw *hw = &pf->hw;
+ u32 reg_idx, bit_idx;
+ int i, tmp, vf_id;
if (!pf->vf)
return;
@@ -748,8 +750,17 @@ void i40e_free_vfs(struct i40e_pf *pf)
kfree(pf->vf);
pf->vf = NULL;
- if (!i40e_vfs_are_assigned(pf))
+ if (!i40e_vfs_are_assigned(pf)) {
pci_disable_sriov(pf->pdev);
+ /* Acknowledge VFLR for all VFS. Without this, VFs will fail to
+ * work correctly when SR-IOV gets re-enabled.
+ */
+ for (vf_id = 0; vf_id < tmp; vf_id++) {
+ reg_idx = (hw->func_caps.vf_base_id + vf_id) / 32;
+ bit_idx = (hw->func_caps.vf_base_id + vf_id) % 32;
+ wr32(hw, I40E_GLGEN_VFLRSTAT(reg_idx), (1 << bit_idx));
+ }
+ }
else
dev_warn(&pf->pdev->dev,
"unable to disable SR-IOV because VFs are assigned.\n");
--
1.8.3.1
^ permalink raw reply related
* [net-next 06/15] i40e: use struct assign instead of memcpy
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Mitch Williams, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mitch Williams <mitch.a.williams@intel.com>
Use struct assignment rather than an expensive memory copy.
Change-Id: I1d18d510774dfd41a9c1250cdef238a4187528f5
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index bed6022..f92404c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1310,7 +1310,7 @@ static int i40e_vc_get_stats_msg(struct i40e_vf *vf, u8 *msg, u16 msglen)
goto error_param;
}
i40e_update_eth_stats(vsi);
- memcpy(&stats, &vsi->eth_stats, sizeof(struct i40e_eth_stats));
+ stats = vsi->eth_stats;
error_param:
/* send the response back to the vf */
--
1.8.3.1
^ permalink raw reply related
* [net-next 05/15] i40e: Do not enable default port on the VEB
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem; +Cc: Greg Rose, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Greg Rose <gregory.v.rose@intel.com>
Enabling the default port on the VEB causes all outgoing traffic from
virtual functions to be copied to the physical function. The default
port is only supposed to be used if you wish to bridge the physical
function to a SW switch such as Open vSwitch or the Linux bridge. That
allows the SW switch to route traffic to VMs that are not using a
virtual function.
Eventually we'll want to implement the ndo_fdb_add, ndo_fdb_del, and
ndo_fdb_dump functions. The ndo_fdb_add function would set the
default port on the VEB in those cases where the MAC/VLAN address
filters have overflowed. Normally we would not want to use it.
Change-Id: I3990f0384fff2840c4e43bc0955dd0b701380852
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d0b2046..c0d7fe6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6903,7 +6903,7 @@ void i40e_veb_release(struct i40e_veb *veb)
**/
static int i40e_add_veb(struct i40e_veb *veb, struct i40e_vsi *vsi)
{
- bool is_default = (vsi->idx == vsi->back->lan_vsi);
+ bool is_default = false;
bool is_cloud = false;
int ret;
--
1.8.3.1
^ permalink raw reply related
* [net-next 03/15] i40e: fix whitespace
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Trivial whitespace fix.
Change-Id: Ib7c70891a33c4b3d200c69367549d0dbdee0f076
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 22fcf94..45bdcca 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -533,6 +533,7 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)
struct i40e_ring *tx_ring = ACCESS_ONCE(vsi->tx_rings[i]);
if (!tx_ring)
continue;
+
dev_info(&pf->pdev->dev,
" tx_rings[%i]: desc = %p\n",
i, tx_ring->desc);
--
1.8.3.1
^ permalink raw reply related
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