* Re: [PATCH 1/5] ISDN-Gigaset: Use kmalloc_array() in two functions
From: SF Markus Elfring @ 2016-09-28 16:38 UTC (permalink / raw)
To: Paul Bolle
Cc: gigaset307x-common, netdev, Karsten Keil, LKML, kernel-janitors,
Julia Lawall
In-Reply-To: <1475062635.16606.27.camel@tiscali.nl>
>> * Multiplications for the size determination of memory allocations
>> indicated that array data structures should be processed.
>> Thus use the corresponding function "kmalloc_array".
>
> Was the current code incorrect?
I suggest to use a safer interface for array allocations.
> What makes kmalloc_array() better?
1. How do you think about the safety checks that this function provides?
2. Will you be also affected by further software evolution here?
2016-07-26
mm: faster kmalloc_array(), kcalloc()
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=91c6a05f72a996bee5133e76374ab3ad7d3b9b72
> I'm not going to change code just because some checker suggests to do so.
The script "checkpatch.pl" can point information out like the following.
WARNING: Prefer kmalloc_array over kmalloc with multiply
>> This issue was detected by using the Coccinelle software.
>
> So? And which coccinelle script was actually used?
How do you think about to look into related information sources?
https://github.com/coccinelle/coccinelle/issues/81
Would you like to experiment any further with an excerpt?
@replacement1@
expression count, target;
type T;
@@
target =
- kmalloc(sizeof(T) * (count)
+ kmalloc_array(count, sizeof(T)
, ...);
@replacement2@
expression count, pointer, target;
@@
target =
- kmalloc(sizeof(*pointer) * (count)
+ kmalloc_array(count, sizeof(*pointer)
, ...);
> I couldn't spot a coccinelle script doing that in the current tree.
This is true for such a software update opportunity.
>> * Replace the specification of a data structure by a pointer dereference
>> to make the corresponding size determination a bit safer according to
>> the Linux coding style convention.
>
> I'm not happy with you mixing this with the above, less trivial, change.
I find that it is a useful combination. - A parameter is adjusted together
with a special function name.
>> - drv->cs = kmalloc(minors * sizeof *drv->cs, GFP_KERNEL);
>> + drv->cs = kmalloc_array(minors, sizeof(*drv->cs), GFP_KERNEL);
>
> For "minors" the same holds as for "channels", above.
>
> And you snuck in a parentheses change. That should have probably been
> merged with 5/5.
Would you prefer to add them in another update step?
Regards,
Markus
^ permalink raw reply
* RE: [PATCH] fs/select: add vmalloc fallback for select(2)
From: David Laight @ 2016-09-28 16:30 UTC (permalink / raw)
To: 'Vlastimil Babka', Nicholas Piggin
Cc: Jason Baron, Hillf Danton, 'Alexander Viro',
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, 'Michal Hocko',
netdev@vger.kernel.org, Eric Dumazet
In-Reply-To: <92d1ec2c-3246-bd1f-eae5-53ca425ab315@suse.cz>
From: Vlastimil Babka
> Sent: 27 September 2016 12:51
...
> Process name suggests it's part of db2 database. It seems it has to implement
> its own interface to select() syscall, because glibc itself seems to have a
> FD_SETSIZE limit of 1024, which is probably why this wasn't an issue for all the
> years...
ISTR the canonical way to increase the size being to set FD_SETSIZE
to a larger value before including any of the headers.
Or doesn't that work with linux and glibc ??
David
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv2 net 0/5] sctp: some fixes of prsctp polices
From: Xin Long @ 2016-09-28 16:29 UTC (permalink / raw)
To: David Miller
Cc: network dev, linux-sctp, Marcelo Ricardo Leitner,
Vladislav Yasevich, daniel
In-Reply-To: <20160928.111716.1982951742537758888.davem@davemloft.net>
>
> That doesn't matter. This happens all the time.
>
> The way you handle this is you submit patch #1 targetting 'net' and
> wait for it to get into 'net' and propagate into 'net-next'. When
> that happens you can submit patches #2-5 as a second patch series
> targetting 'net-next'.
Thanks, will reorganize them.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: phy: Add Wake-on-LAN driver for Microsemi PHYs.
From: Andrew Lunn @ 2016-09-28 16:27 UTC (permalink / raw)
To: Raju Lakkaraju; +Cc: netdev, f.fainelli, Allan.Nielsen
In-Reply-To: <1475064078-22310-2-git-send-email-Raju.Lakkaraju@microsemi.com>
> +#define MSCC_PHY_WOL_MAC_CONTROL 27
> +#define EDGE_RATE_CNTL_POS 5
> +#define EDGE_RATE_CNTL_MASK 0x00E0
This patch does not require these two #defines.
Please indicate in the cover note if the patches depends on other
patches in order to cleanly apply. Or if these patches are going to
conflict with some other patches.
> + reg_val = phy_read(phydev, MSCC_PHY_WOL_MAC_CONTROL);
> + if (wol_conf->wolopts & WAKE_MAGICSECURE)
> + reg_val |= SECURE_ON_ENABLE;
> + else
> + reg_val &= ~SECURE_ON_ENABLE;
> + phy_write(phydev, MSCC_PHY_WOL_MAC_CONTROL, reg_val);
> +
> + if (wol_conf->wolopts & WAKE_MAGICSECURE) {
> + reg_val = wol_conf->sopass[4] << 8;
> + reg_val |= wol_conf->sopass[5];
> + phy_write(phydev, MSCC_PHY_WOL_LOWER_PASSWD, reg_val);
> + reg_val = wol_conf->sopass[2] << 8;
> + reg_val |= wol_conf->sopass[3];
> + phy_write(phydev, MSCC_PHY_WOL_MID_PASSWD, reg_val);
> + reg_val = wol_conf->sopass[0] << 8;
> + reg_val |= wol_conf->sopass[1];
> + phy_write(phydev, MSCC_PHY_WOL_UPPER_PASSWD, reg_val);
> + } else {
> + phy_write(phydev, MSCC_PHY_WOL_LOWER_PASSWD, 0);
> + phy_write(phydev, MSCC_PHY_WOL_MID_PASSWD, 0);
> + phy_write(phydev, MSCC_PHY_WOL_UPPER_PASSWD, 0);
> + }
Wouldn't it be better to set the password, and then enable the
password feature?
I don't know much about WOL. Hopefully Florian will add further
comments.
Andrew
^ permalink raw reply
* Re: [net-next 5/5] PCI: disable FLR for 82579 device
From: Bjorn Helgaas @ 2016-09-28 16:26 UTC (permalink / raw)
To: Neftin, Sasha
Cc: Alexander Duyck, Alex Williamson, Duyck, Alexander H,
Ruinskiy, Dima, Avargil, Raanan, Kirsher, Jeffrey T,
linux-pci@vger.kernel.org, David Miller, Bjorn Helgaas, Netdev,
Neil Horman, sassmann@redhat.com, John Greene,
guru.anbalagane@oracle.com
In-Reply-To: <630A6B92B7EDEB45A87E20D3D286660153E14465@hasmsx109.ger.corp.intel.com>
On Wed, Sep 28, 2016 at 03:33:52PM +0000, Neftin, Sasha wrote:
>
> Since I worked with Sasha on this I will provide a bit of information from what I understand of this bug as well.
>
> On Tue, Sep 27, 2016 at 12:13 PM, Alex Williamson <alex.williamson@redhat.com> wrote:
> > On Tue, 27 Sep 2016 13:17:02 -0500
> > Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> >> On Sun, Sep 25, 2016 at 10:02:43AM +0300, Neftin, Sasha wrote:
> >> > On 9/24/2016 12:05 AM, Jeff Kirsher wrote:
> >> > >On Fri, 2016-09-23 at 09:01 -0500, Bjorn Helgaas wrote:
> >> > >>On Thu, Sep 22, 2016 at 11:39:01PM -0700, Jeff Kirsher wrote:
> >> > >>>From: Sasha Neftin <sasha.neftin@intel.com>
> >> > >>>
> >> > >>>82579 has a problem reattaching itself after the device is detached.
> >> > >>>The bug was reported by Redhat. The suggested fix is to disable
> >> > >>>FLR capability in PCIe configuration space.
> >> > >>>
> >> > >>>Reproduction:
> >> > >>>Attach the device to a VM, then detach and try to attach again.
> >> > >>>
> >> > >>>Fix:
> >> > >>>Disable FLR capability to prevent the 82579 from hanging.
> >> > >>Is there a bugzilla or other reference URL to include here?
> >> > >>Should this be marked for stable?
> >> > >So the author is in Israel, meaning it is their weekend now. I do
> >> > >not believe Sasha monitors email over the weekend, so a response
> >> > >to your questions won't happen for a few days.
> >> > >
> >> > >I tried searching my archives for more information, but had no
> >> > >luck finding any additional information.
> >> > >
>
> I agree that we do probably need to update the patch description since it isn't exactly clear what this is fixing or what was actually broken.
>
> >> > >>>Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> >> > >>>Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> >> > >>>Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >> > >>>---
> >> > >>> drivers/pci/quirks.c | 21 +++++++++++++++++++++
> >> > >>> 1 file changed, 21 insertions(+)
> >> > >>>
> >> > >>>diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> >> > >>>44e0ff3..59fba6e 100644
> >> > >>>--- a/drivers/pci/quirks.c
> >> > >>>+++ b/drivers/pci/quirks.c
> >> > >>>@@ -4431,3 +4431,24 @@ static void quirk_intel_qat_vf_cap(struct
> >> > >>>pci_dev *pdev)
> >> > >>> }
> >> > >>> }
> >> > >>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> >> > >>>quirk_intel_qat_vf_cap);
> >> > >>>+/*
> >> > >>>+ * Workaround FLR issues for 82579
> >> > >>>+ * This code disables the FLR (Function Level Reset) via PCIe,
> >> > >>>+in
> >> > >>>order
> >> > >>>+ * to workaround a bug found while using device passthrough,
> >> > >>>+ where the
> >> > >>>+ * interface would become non-responsive.
> >> > >>>+ * NOTE: the FLR bit is Read/Write Once (RWO) in config space,
> >> > >>>+ so if
> >> > >>>+ * the BIOS or kernel writes this register * then this
> >> > >>>+ workaround will
> >> > >>>+ * not work.
> >> > >>This doesn't sound like a root cause. Is the issue a hardware
> >> > >>erratum? Linux PCI core bug? VFIO bug? Device firmware bug?
> >> > >>
> >> > >>The changelog suggests that the problem only affects passthrough,
> >> > >>which suggests some sort of kernel bug related to how passthrough
> >> > >>is implemented.
> >>
> >> If this bug affects all scenarios, not just passthrough, the
> >> changelog should not mention passthrough.
> >>
> >> > >>>+ */
> >> > >>>+static void quirk_intel_flr_cap_dis(struct pci_dev *dev) {
> >> > >>>+ int pos = pci_find_capability(dev, PCI_CAP_ID_AF);
> >> > >>>+ if (pos) {
> >> > >>>+ u8 cap;
> >> > >>>+ pci_read_config_byte(dev, pos + PCI_AF_CAP, &cap);
> >> > >>>+ cap = cap & (~PCI_AF_CAP_FLR);
> >> > >>>+ pci_write_config_byte(dev, pos + PCI_AF_CAP, cap);
> >> > >>>+ }
> >> > >>>+}
> >> > >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502,
> >> > >>>quirk_intel_flr_cap_dis);
> >> > >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503,
> >> > >>>quirk_intel_flr_cap_dis);
> >> > >>>--
> >> > >>>2.7.4
> >> > >>>
> >> > >>>--
> >> > >>>To unsubscribe from this list: send the line "unsubscribe
> >> > >>>linux-pci" in the body of a message to majordomo@vger.kernel.org
> >> > >>>More majordomo info at
> >> > >>>http://vger.kernel.org/majordomo-info.html
> >> >
> >> > Hello,
> >> >
> >> > Original bugzilla thread could be found here:
> >> > https://bugzilla.redhat.com/show_bug.cgi?format=multiple&id=966840
> >>
> >> That bugzilla is private and I can't read it.
> >
> > Hmm, I can, but I don't see anything in it that supports this. Is
> > that really the right bz? It's the right hardware, but has all sorts
> > of FUD about the version of various other components in the stack.
>
> It looks like we had a local copy of the bugzilla saved here, though it only goes up to comment 13 which is where I think we started working this on our side. I believe what this patch is attempting to resolve is related to comment 8 where the driver returned "probe of
> 0000:00:19.0 failed with error ‐2" instead of correctly probing the interface.
>
> So the bug as reported was that e1000e had a problem reattaching itself to the PHY after it was attached to a VM. Sasha, please feel free to correct me if I have this bit wrong. I had been told the problem was the FLR functionality wasn't fully implemented so that was why they were wanting to defeature it. I had assumed that there was support for a reset on D0->D3->D0, but I just realized that probably isn't the case since it looks like my local system sets the NoSoftRST bit.
>
> >> > This is our HW bug, exist only in 82579 devices. More new devices
> >> > have no such problem. We have found root cause and suggested this
> >> > solution.
> >>
> >> Is there an erratum you can reference?
> >>
> >> > This solution should work for a 95% of cases, so I do not think
> >> > that this is fragile. For another cases possible solution is get up
> >> > working system and manually disable FLR, before VM start use our
> >> > adapter.
> >>
> >> I don't think a 95% solution is sufficient. Can you use the
> >> pci_dev_specific_reset() framework to make a 100% solution?
>
> I can try working with Sasha on this to see what we can do.
>
> > Right, plus when this does work I suspect it removes the one mechanism
> > we have to reset the device, which depending on how obscure the
> > failure scenario is, isn't a clear cut improvement for device assignment.
> > Thanks,
> >
> > Alex
>
> I'll work with Sasha to see what we can do. Odds are there is some sort of problem between the MAC/PHY that needs to be resolved when we perform the function level reset so we will probably need to add code so that we reset the part and re-establish the link with the PHY after the reset.
>
> - Alex
>
>
> Hello,
> I would like clarify a few points.
>
> 1. 82579 client devices do not support functional level reset (FLR).
> Unfortunately, it advertises FLR capability - this is a bug. So,
> when VM software even accidentally tries to access FLR, that causes
> our device to hang. To eliminate this problem we decided to disable
> FLR via drivers/pci/quirks.c
That makes sense. To me, the problem is that writing the AF
capability to disable FLR is unreliable. We can't tell whether BIOS
has already written it, and there's no way to debug the corner cases
where BIOS *has* written it and the device doesn't reset correctly.
Maybe it is in fact weird for a BIOS to write that, and the corner
case will never arise. But I have no way of knowing that, and I don't
like making assumptions about what a BIOS will or won't do.
Here are two ideas:
1) Implement a device-specific reset quirk. The problem here is
that you'd have to duplicate the body of __pci_dev_reset(), omitting
the pcie_flr() and pci_af_flr() calls. This might be a maintenance
issue because future changes to __pci_dev_reset() should also be
made to the quirk.
2) Add some sort of "broken_flr_reset" bit in struct pci_dev, set it
with a quirk, and change pci_af_flr() to return -ENOTTY when it is
set.
> In previous e-mail I wrote that the solution should work for a 95% of cases. Cases where it might not work if BIOS writes FLR capability before the OS (In 82579 device FLR capability is RWO - read write once). But this is a very weird scenario, so probably the workaround will work close to 100%. Also, I would like to add that we have run lots of tests over 82579 in our lab and ensured that all flows work properly.
>
> 2. D0->D3-D0 flow is not affected and is in fact supported by 82579.
>
> 3. I will look for an erratum explaining this.
>
> Thanks,
> Sasha
>
>
>
>
>
^ permalink raw reply
* Re: [PATCH v4 net-next] net: phy: Add Edge-rate driver for Microsemi PHYs.
From: Andrew Lunn @ 2016-09-28 16:16 UTC (permalink / raw)
To: Raju Lakkaraju; +Cc: netdev, devicetree, f.fainelli, Allan.Nielsen
In-Reply-To: <1475062449-22168-1-git-send-email-Raju.Lakkaraju@microsemi.com>
> +Optional properties:
> +- vsc8531,vddmac : The vddmac in mV.
> +- vsc8531,edge-slowdown : % the edge should be slowed down relative to
> + the fastest possible edge time. Native sign
> + need not enter.
> + Edge rate sets the drive strength of the MAC
> + interface output signals. Changing the drive
> + strength will affect the edge rate of the output
> + signal. The goal of this setting is to help
> + reduce electrical emission (EMI) by being able
> + to reprogram drive strength and in effect slow
> + down the edge rate if desired. Table 5 shows the
Hi Raju
There is no table five here? Is that a reference to a data sheet
table?
> +Example:
> +
> + vsc8531_0: ethernet-phy@0 {
> + compatible = "ethernet-phy-id0007.0570";
> + vsc8531,vddmac = /bits/ 16 <MSCC_VDDMAC_3300>;
> + vsc8531,edge-slowdown = /bits/ 8 <17>;
No, real values please:
vsc8531,vddmac = <2000>;
vsc8531,edge-slowdown = <21>;
The driver should then do the maths to figure out the nearest magic
value to write to the register, or complain the settings are out of
range with an -EINVAL.
FYI: No floating point maths are allowed in the kernel.
Andrew
^ permalink raw reply
* Local variable ordering (was Re: [PATCH v5 0/7] Reduce cache miss for snmp_fold_field)
From: Edward Cree @ 2016-09-28 15:58 UTC (permalink / raw)
To: hejianet
Cc: netdev, linux-sctp, linux-kernel, kuznet, jmorris, yoshfuji,
kaber, vyasevich, nhorman, steffen.klassert, herbert,
marcelo.leitner
In-Reply-To: <c44daa11-dc57-84fa-35cc-40ee0fe293f3@gmail.com>
On 28/09/16 14:45, hejianet wrote:
>
>
> On 9/28/16 5:08 PM, David Miller wrote:
>> From: Jia He <hejianet@gmail.com>
>> Date: Wed, 28 Sep 2016 14:22:21 +0800
>>
>>> v5:
>>> - order local variables from longest to shortest line
>> I still see many cases where this problem still exists. Please
>> do not resubmit this patch series until you fix all of them.
>>
>> Patch #2:
>>
>> -static int snmp_seq_show(struct seq_file *seq, void *v)
>> +static int snmp_seq_show_ipstats(struct seq_file *seq, void *v)
>> {
>> int i;
>> + u64 buff64[IPSTATS_MIB_MAX];
>> struct net *net = seq->private;
>>
>> The order should be "net" then "buff64" then "i".
> Sorry for my bad eyesight and quick hand :(
> B.R.
> Jia
A few months back I wrote a script to help find these:
https://github.com/ecree-solarflare/xmastree
It can check a source file or a patch.
Posting in the hope that people will find it useful.
-Ed
^ permalink raw reply
* Re: [PATCHv2 net 0/5] sctp: some fixes of prsctp polices
From: marcelo.leitner @ 2016-09-28 15:44 UTC (permalink / raw)
To: David Miller; +Cc: lucien.xin, netdev, linux-sctp, vyasevich, daniel
In-Reply-To: <20160928.111716.1982951742537758888.davem@davemloft.net>
On Wed, Sep 28, 2016 at 11:17:16AM -0400, David Miller wrote:
> From: Xin Long <lucien.xin@gmail.com>
> Date: Wed, 28 Sep 2016 20:35:40 +0800
>
> >>
> >>> This patchset is to improve some codes about prsctp polices, and also
> >>> to fix some issues.
> >>>
> >>> v1->v2:
> >>> - wrap the check of chunk->sent_count in a macro:
> >>> sctp_chunk_retransmitted in patch 2/5.
> >>
> >> This series is a mix of bug fixes (patch #1) which should be targetting
> >> 'net' and simplifications/cleanups (patch #2-5) which should be targetting
> >> 'net-next'.
> >>
> >> Please do not mix things up like this, and submit patches targetting
> >> the appropriate tree.
> >>
> > understand, I wanted to divide them.
> > but this patchset is special, the following patches
> > depend on patch 1/5.
>
> That doesn't matter. This happens all the time.
>
> The way you handle this is you submit patch #1 targetting 'net' and
> wait for it to get into 'net' and propagate into 'net-next'. When
> that happens you can submit patches #2-5 as a second patch series
> targetting 'net-next'.
Thanks for your patience for explaining this, David, appreciated.
Marcelo
^ permalink raw reply
* [PATCH net-next] net: do not export sk_stream_write_space
From: Eric Dumazet @ 2016-09-28 15:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell
From: Eric Dumazet <edumazet@google.com>
Since commit 900f65d361d3 ("tcp: move duplicate code from
tcp_v4_init_sock()/tcp_v6_init_sock()") we no longer need
to export sk_stream_write_space()
From: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
net/core/stream.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/core/stream.c b/net/core/stream.c
index 159516a11b7e84f7e64e0acfa2dd55dab82307bd..1086c8b280a868101df7b44691eccac40bd7b3e1 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -43,7 +43,6 @@ void sk_stream_write_space(struct sock *sk)
rcu_read_unlock();
}
}
-EXPORT_SYMBOL(sk_stream_write_space);
/**
* sk_stream_wait_connect - Wait for a socket to get into the connected state
^ permalink raw reply related
* Re: [PATCH nf-next v3 1/2] netfilter: Fix potential null pointer dereference
From: Aaron Conole @ 2016-09-28 15:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, netdev, Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <1475076603.28155.105.camel@edumazet-glaptop3.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> On Wed, 2016-09-28 at 10:56 -0400, Aaron Conole wrote:
>> Eric Dumazet <eric.dumazet@gmail.com> writes:
>>
>> > On Wed, 2016-09-28 at 09:12 -0400, Aaron Conole wrote:
>> >> It's possible for nf_hook_entry_head to return NULL. If two
>> >> nf_unregister_net_hook calls happen simultaneously with a single hook
>> >> entry in the list, both will enter the nf_hook_mutex critical section.
>> >> The first will successfully delete the head, but the second will see
>> >> this NULL pointer and attempt to dereference.
>> >>
>> >> This fix ensures that no null pointer dereference could occur when such
>> >> a condition happens.
>> >>
>> >> Signed-off-by: Aaron Conole <aconole@bytheb.org>
>> >> ---
>> >> net/netfilter/core.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
>> >> index 360c63d..e58e420 100644
>> >> --- a/net/netfilter/core.c
>> >> +++ b/net/netfilter/core.c
>> >> @@ -160,7 +160,7 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
>> >>
>> >> mutex_lock(&nf_hook_mutex);
>> >> hooks_entry = nf_hook_entry_head(net, reg);
>> >> - if (hooks_entry->orig_ops == reg) {
>> >> + if (hooks_entry && hooks_entry->orig_ops == reg) {
>> >> nf_set_hooks_head(net, reg,
>> >> nf_entry_dereference(hooks_entry->next));
>> >> goto unlock;
>> >
>> > When was the bug added exactly ?
>>
>> Sunday, on the nf-next tree.
>>
>> > For all bug fixes, you need to add a Fixes: tag.
>> >
>> > Like :
>> >
>> > Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
>>
>> I would but it's in nf-next tree, and I'm not sure how pulls go. If
>> they are done via patch imports, then the sha sums will be wrong and the
>> commit message will be misleading. If the sums are preserved, then I
>> can resubmit with this information.
>>
>
> I gave the (12 digits) sha-1 as present in David Miller net-next tree.
>
> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=e3b37f11e6e4e6b6f02cc762f182ce233d2c1c9d
>
>
> This wont change, because David never rebases his tree under normal
> operations.
>
> Thanks.
Thank you very much, Eric. I've reposted.
-Aaron
^ permalink raw reply
* [PATCH nf-next v4 2/2] nf_set_hooks_head: accommodate different kconfig
From: Aaron Conole @ 2016-09-28 15:35 UTC (permalink / raw)
To: netfilter-devel, netdev; +Cc: Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <1475076915-5920-1-git-send-email-aconole@bytheb.org>
When CONFIG_NETFILTER_INGRESS is unset (or no), we need to handle
the request for registration properly by dropping the hook. This
releases the entry during the set.
Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
Signed-off-by: Aaron Conole <aconole@bytheb.org>
---
net/netfilter/core.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index e58e420..61e8a9d 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -90,10 +90,12 @@ static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
{
switch (reg->pf) {
case NFPROTO_NETDEV:
+#ifdef CONFIG_NETFILTER_INGRESS
/* We already checked in nf_register_net_hook() that this is
* used from ingress.
*/
rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
+#endif
break;
default:
rcu_assign_pointer(net->nf.hooks[reg->pf][reg->hooknum],
@@ -107,10 +109,15 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
struct nf_hook_entry *hooks_entry;
struct nf_hook_entry *entry;
- if (reg->pf == NFPROTO_NETDEV &&
- (reg->hooknum != NF_NETDEV_INGRESS ||
- !reg->dev || dev_net(reg->dev) != net))
- return -EINVAL;
+ if (reg->pf == NFPROTO_NETDEV) {
+#ifndef CONFIG_NETFILTER_INGRESS
+ if (reg->hooknum == NF_NETDEV_INGRESS)
+ return -EOPNOTSUPP;
+#endif
+ if (reg->hooknum != NF_NETDEV_INGRESS ||
+ !reg->dev || dev_net(reg->dev) != net)
+ return -EINVAL;
+ }
entry = kmalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
--
2.7.4
^ permalink raw reply related
* [PATCH nf-next v4 1/2] netfilter: Fix potential null pointer dereference
From: Aaron Conole @ 2016-09-28 15:35 UTC (permalink / raw)
To: netfilter-devel, netdev; +Cc: Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <1475076915-5920-1-git-send-email-aconole@bytheb.org>
It's possible for nf_hook_entry_head to return NULL. If two
nf_unregister_net_hook calls happen simultaneously with a single hook
entry in the list, both will enter the nf_hook_mutex critical section.
The first will successfully delete the head, but the second will see
this NULL pointer and attempt to dereference.
This fix ensures that no null pointer dereference could occur when such
a condition happens.
Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
Signed-off-by: Aaron Conole <aconole@bytheb.org>
---
net/netfilter/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 360c63d..e58e420 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -160,7 +160,7 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
mutex_lock(&nf_hook_mutex);
hooks_entry = nf_hook_entry_head(net, reg);
- if (hooks_entry->orig_ops == reg) {
+ if (hooks_entry && hooks_entry->orig_ops == reg) {
nf_set_hooks_head(net, reg,
nf_entry_dereference(hooks_entry->next));
goto unlock;
--
2.7.4
^ permalink raw reply related
* [PATCH nf-next v4 0/2] fixes for recent nf_compact hooks
From: Aaron Conole @ 2016-09-28 15:35 UTC (permalink / raw)
To: netfilter-devel, netdev; +Cc: Florian Westphal, Pablo Neira Ayuso
Two possible error conditions were caught during an extended testing
session, and by a build robot. These patches fix the two issues (a
missing handler when config is changed, and a potential NULL
dereference).
Aaron Conole (2):
netfilter: Fix potential null pointer dereference
nf_set_hooks_head: acommodate different kconfig
net/netfilter/core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--
2.5.5
^ permalink raw reply
* RE: [net-next 5/5] PCI: disable FLR for 82579 device
From: Neftin, Sasha @ 2016-09-28 15:33 UTC (permalink / raw)
To: Alexander Duyck, Alex Williamson, Duyck, Alexander H,
Ruinskiy, Dima, Avargil, Raanan, Neftin, Sasha
Cc: Bjorn Helgaas, Kirsher, Jeffrey T, linux-pci@vger.kernel.org,
David Miller, Bjorn Helgaas, Netdev, Neil Horman,
sassmann@redhat.com, John Greene, guru.anbalagane@oracle.com
In-Reply-To: <CAKgT0UczaC-5z9GB74xhU11iWLu5_n75najKzBZcvk1T3C16Og@mail.gmail.com>
Since I worked with Sasha on this I will provide a bit of information from what I understand of this bug as well.
On Tue, Sep 27, 2016 at 12:13 PM, Alex Williamson <alex.williamson@redhat.com> wrote:
> On Tue, 27 Sep 2016 13:17:02 -0500
> Bjorn Helgaas <helgaas@kernel.org> wrote:
>
>> On Sun, Sep 25, 2016 at 10:02:43AM +0300, Neftin, Sasha wrote:
>> > On 9/24/2016 12:05 AM, Jeff Kirsher wrote:
>> > >On Fri, 2016-09-23 at 09:01 -0500, Bjorn Helgaas wrote:
>> > >>On Thu, Sep 22, 2016 at 11:39:01PM -0700, Jeff Kirsher wrote:
>> > >>>From: Sasha Neftin <sasha.neftin@intel.com>
>> > >>>
>> > >>>82579 has a problem reattaching itself after the device is detached.
>> > >>>The bug was reported by Redhat. The suggested fix is to disable
>> > >>>FLR capability in PCIe configuration space.
>> > >>>
>> > >>>Reproduction:
>> > >>>Attach the device to a VM, then detach and try to attach again.
>> > >>>
>> > >>>Fix:
>> > >>>Disable FLR capability to prevent the 82579 from hanging.
>> > >>Is there a bugzilla or other reference URL to include here?
>> > >>Should this be marked for stable?
>> > >So the author is in Israel, meaning it is their weekend now. I do
>> > >not believe Sasha monitors email over the weekend, so a response
>> > >to your questions won't happen for a few days.
>> > >
>> > >I tried searching my archives for more information, but had no
>> > >luck finding any additional information.
>> > >
I agree that we do probably need to update the patch description since it isn't exactly clear what this is fixing or what was actually broken.
>> > >>>Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
>> > >>>Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>> > >>>Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> > >>>---
>> > >>> drivers/pci/quirks.c | 21 +++++++++++++++++++++
>> > >>> 1 file changed, 21 insertions(+)
>> > >>>
>> > >>>diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
>> > >>>44e0ff3..59fba6e 100644
>> > >>>--- a/drivers/pci/quirks.c
>> > >>>+++ b/drivers/pci/quirks.c
>> > >>>@@ -4431,3 +4431,24 @@ static void quirk_intel_qat_vf_cap(struct
>> > >>>pci_dev *pdev)
>> > >>> }
>> > >>> }
>> > >>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
>> > >>>quirk_intel_qat_vf_cap);
>> > >>>+/*
>> > >>>+ * Workaround FLR issues for 82579
>> > >>>+ * This code disables the FLR (Function Level Reset) via PCIe,
>> > >>>+in
>> > >>>order
>> > >>>+ * to workaround a bug found while using device passthrough,
>> > >>>+ where the
>> > >>>+ * interface would become non-responsive.
>> > >>>+ * NOTE: the FLR bit is Read/Write Once (RWO) in config space,
>> > >>>+ so if
>> > >>>+ * the BIOS or kernel writes this register * then this
>> > >>>+ workaround will
>> > >>>+ * not work.
>> > >>This doesn't sound like a root cause. Is the issue a hardware
>> > >>erratum? Linux PCI core bug? VFIO bug? Device firmware bug?
>> > >>
>> > >>The changelog suggests that the problem only affects passthrough,
>> > >>which suggests some sort of kernel bug related to how passthrough
>> > >>is implemented.
>>
>> If this bug affects all scenarios, not just passthrough, the
>> changelog should not mention passthrough.
>>
>> > >>>+ */
>> > >>>+static void quirk_intel_flr_cap_dis(struct pci_dev *dev) {
>> > >>>+ int pos = pci_find_capability(dev, PCI_CAP_ID_AF);
>> > >>>+ if (pos) {
>> > >>>+ u8 cap;
>> > >>>+ pci_read_config_byte(dev, pos + PCI_AF_CAP, &cap);
>> > >>>+ cap = cap & (~PCI_AF_CAP_FLR);
>> > >>>+ pci_write_config_byte(dev, pos + PCI_AF_CAP, cap);
>> > >>>+ }
>> > >>>+}
>> > >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502,
>> > >>>quirk_intel_flr_cap_dis);
>> > >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503,
>> > >>>quirk_intel_flr_cap_dis);
>> > >>>--
>> > >>>2.7.4
>> > >>>
>> > >>>--
>> > >>>To unsubscribe from this list: send the line "unsubscribe
>> > >>>linux-pci" in the body of a message to majordomo@vger.kernel.org
>> > >>>More majordomo info at
>> > >>>http://vger.kernel.org/majordomo-info.html
>> >
>> > Hello,
>> >
>> > Original bugzilla thread could be found here:
>> > https://bugzilla.redhat.com/show_bug.cgi?format=multiple&id=966840
>>
>> That bugzilla is private and I can't read it.
>
> Hmm, I can, but I don't see anything in it that supports this. Is
> that really the right bz? It's the right hardware, but has all sorts
> of FUD about the version of various other components in the stack.
It looks like we had a local copy of the bugzilla saved here, though it only goes up to comment 13 which is where I think we started working this on our side. I believe what this patch is attempting to resolve is related to comment 8 where the driver returned "probe of
0000:00:19.0 failed with error ‐2" instead of correctly probing the interface.
So the bug as reported was that e1000e had a problem reattaching itself to the PHY after it was attached to a VM. Sasha, please feel free to correct me if I have this bit wrong. I had been told the problem was the FLR functionality wasn't fully implemented so that was why they were wanting to defeature it. I had assumed that there was support for a reset on D0->D3->D0, but I just realized that probably isn't the case since it looks like my local system sets the NoSoftRST bit.
>> > This is our HW bug, exist only in 82579 devices. More new devices
>> > have no such problem. We have found root cause and suggested this
>> > solution.
>>
>> Is there an erratum you can reference?
>>
>> > This solution should work for a 95% of cases, so I do not think
>> > that this is fragile. For another cases possible solution is get up
>> > working system and manually disable FLR, before VM start use our
>> > adapter.
>>
>> I don't think a 95% solution is sufficient. Can you use the
>> pci_dev_specific_reset() framework to make a 100% solution?
I can try working with Sasha on this to see what we can do.
> Right, plus when this does work I suspect it removes the one mechanism
> we have to reset the device, which depending on how obscure the
> failure scenario is, isn't a clear cut improvement for device assignment.
> Thanks,
>
> Alex
I'll work with Sasha to see what we can do. Odds are there is some sort of problem between the MAC/PHY that needs to be resolved when we perform the function level reset so we will probably need to add code so that we reset the part and re-establish the link with the PHY after the reset.
- Alex
Hello,
I would like clarify a few points.
1. 82579 client devices do not support functional level reset (FLR). Unfortunately, it advertises FLR capability - this is a bug. So, when VM software even accidentally tries to access FLR, that causes our device to hang. To eliminate this problem we decided to disable FLR via drivers/pci/quirks.c
In previous e-mail I wrote that the solution should work for a 95% of cases. Cases where it might not work if BIOS writes FLR capability before the OS (In 82579 device FLR capability is RWO - read write once). But this is a very weird scenario, so probably the workaround will work close to 100%. Also, I would like to add that we have run lots of tests over 82579 in our lab and ensured that all flows work properly.
2. D0->D3-D0 flow is not affected and is in fact supported by 82579.
3. I will look for an erratum explaining this.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH nf-next v3 1/2] netfilter: Fix potential null pointer dereference
From: Eric Dumazet @ 2016-09-28 15:30 UTC (permalink / raw)
To: Aaron Conole; +Cc: netfilter-devel, netdev, Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <f7ty42chy3k.fsf@redhat.com>
On Wed, 2016-09-28 at 10:56 -0400, Aaron Conole wrote:
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>
> > On Wed, 2016-09-28 at 09:12 -0400, Aaron Conole wrote:
> >> It's possible for nf_hook_entry_head to return NULL. If two
> >> nf_unregister_net_hook calls happen simultaneously with a single hook
> >> entry in the list, both will enter the nf_hook_mutex critical section.
> >> The first will successfully delete the head, but the second will see
> >> this NULL pointer and attempt to dereference.
> >>
> >> This fix ensures that no null pointer dereference could occur when such
> >> a condition happens.
> >>
> >> Signed-off-by: Aaron Conole <aconole@bytheb.org>
> >> ---
> >> net/netfilter/core.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> >> index 360c63d..e58e420 100644
> >> --- a/net/netfilter/core.c
> >> +++ b/net/netfilter/core.c
> >> @@ -160,7 +160,7 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
> >>
> >> mutex_lock(&nf_hook_mutex);
> >> hooks_entry = nf_hook_entry_head(net, reg);
> >> - if (hooks_entry->orig_ops == reg) {
> >> + if (hooks_entry && hooks_entry->orig_ops == reg) {
> >> nf_set_hooks_head(net, reg,
> >> nf_entry_dereference(hooks_entry->next));
> >> goto unlock;
> >
> > When was the bug added exactly ?
>
> Sunday, on the nf-next tree.
>
> > For all bug fixes, you need to add a Fixes: tag.
> >
> > Like :
> >
> > Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
>
> I would but it's in nf-next tree, and I'm not sure how pulls go. If
> they are done via patch imports, then the sha sums will be wrong and the
> commit message will be misleading. If the sums are preserved, then I
> can resubmit with this information.
>
I gave the (12 digits) sha-1 as present in David Miller net-next tree.
https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=e3b37f11e6e4e6b6f02cc762f182ce233d2c1c9d
This wont change, because David never rebases his tree under normal
operations.
Thanks.
^ permalink raw reply
* Re: [PATCH] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
From: David Miller @ 2016-09-28 15:21 UTC (permalink / raw)
To: zenczykowski; +Cc: hannes, ek, lorenzo, netdev
In-Reply-To: <CAHo-Oowk-YN2bHkWWKe_yAWsXQtybu3CZ+beXTiAO9914vouyg@mail.gmail.com>
From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Wed, 28 Sep 2016 22:23:10 +0900
> Anyway, enough, I give up, this isn't worth my time, and it's also not
> worth your time.
> I removed the dependency from the other patches and squashed them all into
> 1 to make reviewing easier.
Splitting things up is sometimes warranted, but you have to keep
together the things that strongly need each other.
And new facilities require examples of their use. They really do.
I simply can't look at your patch and empty commit message and figure
out why you needed to do what you were doing.
Any developer should be able to look at a patch being proposed and be
able to understand it standing upon on it's own. This means they
shouldn't have to know what happened in this discussion thread or that
one in order to evaluate and audit the patch properly.
^ permalink raw reply
* Re: [PATCHv2 net 0/5] sctp: some fixes of prsctp polices
From: David Miller @ 2016-09-28 15:17 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, vyasevich, daniel
In-Reply-To: <CADvbK_eKrZaGf0uDsCBdkWCw9e51W-HVL=_WLsR4RaSKjNmqqA@mail.gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Wed, 28 Sep 2016 20:35:40 +0800
>>
>>> This patchset is to improve some codes about prsctp polices, and also
>>> to fix some issues.
>>>
>>> v1->v2:
>>> - wrap the check of chunk->sent_count in a macro:
>>> sctp_chunk_retransmitted in patch 2/5.
>>
>> This series is a mix of bug fixes (patch #1) which should be targetting
>> 'net' and simplifications/cleanups (patch #2-5) which should be targetting
>> 'net-next'.
>>
>> Please do not mix things up like this, and submit patches targetting
>> the appropriate tree.
>>
> understand, I wanted to divide them.
> but this patchset is special, the following patches
> depend on patch 1/5.
That doesn't matter. This happens all the time.
The way you handle this is you submit patch #1 targetting 'net' and
wait for it to get into 'net' and propagate into 'net-next'. When
that happens you can submit patches #2-5 as a second patch series
targetting 'net-next'.
^ permalink raw reply
* Re: [PATCH RFC 0/4] xfs: Transmit flow steering
From: Rick Jones @ 2016-09-28 15:13 UTC (permalink / raw)
To: Tom Herbert, davem, netdev; +Cc: kernel-team, Balliet, Drew, Juerg Haefliger
In-Reply-To: <1472601634-531498-1-git-send-email-tom@herbertland.com>
Here is a quick look at performance tests for the result of trying the
prototype fix for the packet reordering problem with VMs sending over
an XPS-configured NIC. In particular, the Emulex/Avago/Broadcom
Skyhawk. The fix was applied to a 4.4 kernel.
Before: 3884 Mbit/s
After: 8897 Mbit/s
That was from a VM on a node with a Skyhawk and 2 E5-2640 processors
to baremetal E5-2640 with a BE3. Physical MTU was 1500, the VM's
vNIC's MTU was 1400. Systems were HPE ProLiants in OS Control Mode
for power management, with the "performance" frequency governor
loaded. An OpenStack Mitaka setup with Distributed Virtual Router.
We had some other NIC types in the setup as well. XPS was also
enabled on the ConnectX3-Pro. It was not enabled on the 82599ES (a
function of the kernel being used, which had it disabled from the
first reports of XPS negatively affecting VM traffic at the beginning
of the year)
Average Mbit/s From NIC type To Bare Metal BE3:
NIC Type,
CPU on VM Host Before After
------------------------------------------------
ConnectX-3 Pro,E5-2670v3 9224 9271
BE3, E5-2640 9016 9022
82599, E5-2640 9192 9003
BCM57840, E5-2640 9213 9153
Skyhawk, E5-2640 3884 8897
For completeness:
Average Mbit/s To NIC type from Bare Metal BE3:
NIC Type,
CPU on VM Host Before After
------------------------------------------------
ConnectX-3 Pro,E5-2670v3 9322 9144
BE3, E5-2640 9074 9017
82599, E5-2640 8670 8564
BCM57840, E5-2640 2468 * 7979
Skyhawk, E5-2640 8897 9269
* This is the Busted bnx2x NIC FW GRO implementation issue. It was
not visible in the "After" because the system was setup to disable
the NIC FW GRO by the time it booted on the fix kernel.
Average Transactions/s Between NIC type and Bare Metal BE3:
NIC Type,
CPU on VM Host Before After
------------------------------------------------
ConnectX-3 Pro,E5-2670v3 12421 12612
BE3, E5-2640 8178 8484
82599, E5-2640 8499 8549
BCM57840, E5-2640 8544 8560
Skyhawk, E5-2640 8537 8701
happy benchmarking,
Drew Balliet
Jeurg Haefliger
rick jones
The semi-cooked results with additional statistics:
554M - BE3
544+M - ConnectX-3 Pro
560M - 82599ES
630M - BCM57840
650M - Skyhawk
(substitute is simply replacing a system name with the model of NIC and CPU)
Bulk To (South) and From (North) VM, Before:
$ ../substitute.sh
vxlan_554m_control_performance_gvnr_dvr_northsouth_stream.log |
~/netperf2_trunk/doc/examples/parse_single_stream.py -r -5 -f 1 -f 3 -f
4 -f 7 -f 8
Field1,Field3,Field4,Field7,Field8,Min,P10,Median,Average,P90,P99,Max,Count
North,560M,E5-2640,554FLB,E5-2640,8148.090,9048.830,9235.400,9192.868,9315.980,9338.845,9339.500,113
North,630M,E5-2640,554FLB,E5-2640,8909.980,9113.238,9234.750,9213.140,9299.442,9336.206,9337.830,47
North,544+M,E5-2670v3,554FLB,E5-2640,9013.740,9182.546,9229.620,9224.025,9264.036,9299.206,9301.970,99
North,650M,E5-2640,554FLB,E5-2640,3187.680,3393.724,3796.160,3884.765,4405.096,4941.391,4956.300,129
North,554M,E5-2640,554FLB,E5-2640,8700.930,8855.768,9026.030,9016.061,9158.846,9213.687,9226.150,135
South,554FLB,E5-2640,560M,E5-2640,7754.350,8193.114,8718.540,8670.612,9026.436,9262.355,9285.010,113
South,554FLB,E5-2640,630M,E5-2640,1897.660,2068.290,2514.430,2468.323,2787.162,2942.934,2957.250,53
South,554FLB,E5-2640,544+M,E5-2670v3,9298.260,9314.432,9323.220,9322.207,9328.324,9330.704,9331.080,100
South,554FLB,E5-2640,650M,E5-2640,8407.050,8907.136,9304.390,9206.776,9321.320,9325.347,9326.410,103
South,554FLB,E5-2640,554M,E5-2640,7844.900,8632.530,9199.385,9074.535,9308.070,9319.224,9322.360,132
0 too-short lines ignored.
Bulk To (South) and From (North) VM, After:
$ ../substitute.sh
vxlan_554m_control_performance_gvnr_xpsfix_dvr_northsouth_stream.log |
~/netperf2_trunk/doc/examples/parse_single_stream.py -r -5 -f 1 -f 3 -f
4 -f 7 -f 8
Field1,Field3,Field4,Field7,Field8,Min,P10,Median,Average,P90,P99,Max,Count
North,560M,E5-2640,554FLB,E5-2640,7576.790,8213.890,9182.870,9003.190,9295.975,9315.878,9318.160,36
North,630M,E5-2640,554FLB,E5-2640,8811.800,8924.000,9206.660,9153.076,9306.287,9315.152,9315.790,12
North,544+M,E5-2670v3,554FLB,E5-2640,9135.990,9228.520,9277.465,9271.875,9324.545,9339.604,9339.780,46
North,650M,E5-2640,554FLB,E5-2640,8133.420,8483.340,8995.040,8897.779,9129.056,9165.230,9165.860,43
North,554M,E5-2640,554FLB,E5-2640,8438.390,8879.150,9048.590,9022.813,9181.540,9248.650,9297.660,101
South,554FLB,E5-2640,630M,E5-2640,7347.120,7592.565,7951.325,7979.951,8365.400,8575.837,8579.890,16
South,554FLB,E5-2640,560M,E5-2640,7719.510,8044.496,8602.750,8564.741,9172.824,9248.686,9259.070,45
South,554FLB,E5-2640,544+M,E5-2670v3,8838.660,8907.402,9112.335,9114.040,9326.510,9329.062,9329.990,52
South,554FLB,E5-2640,650M,E5-2640,8699.660,9204.378,9307.940,9269.755,9321.060,9328.007,9331.370,58
South,554FLB,E5-2640,554M,E5-2640,7893.310,8483.182,9111.070,9017.273,9314.984,9322.822,9326.750,103
Request/Response Before:
$ ../substitute.sh
vxlan_554m_control_performance_gvnr_dvr_northsouth_rr.log |
~/netperf2_trunk/doc/examples/parse_single_stream.py -r -5 -f 1 -f 3 -f
4 -f 7 -f 8
Field1,Field3,Field4,Field7,Field8,Min,P10,Median,Average,P90,P99,Max,Count
RR,554FLB,E5-2640,560M,E5-2640,6971.540,8356.420,8515.275,8499.397,8717.740,9023.489,9114.990,186
RR,554FLB,E5-2640,630M,E5-2640,7538.530,8461.090,8532.970,8544.005,8665.414,8895.486,9003.330,119
RR,554FLB,E5-2640,544+M,E5-2670v3,8825.970,12279.268,12504.175,12421.076,12646.219,12742.536,12831.490,238
RR,554FLB,E5-2640,650M,E5-2640,7152.220,8425.368,8535.370,8537.314,8757.256,9046.692,9128.290,219
Request/Response After:
$ ../substitute.sh
vxlan_554m_control_performance_gvnr_xpsfix_dvr_northsouth_rr.log |
~/netperf2_trunk/doc/examples/parse_single_stream.py -r -5 -f 1 -f 3 -f
4 -f 7 -f 8
Field1,Field3,Field4,Field7,Field8,Min,P10,Median,Average,P90,P99,Max,Count
RR,554FLB,E5-2640,560M,E5-2640,7385.910,8411.654,8568.280,8549.659,8724.006,8874.092,8971.860,85
RR,554FLB,E5-2640,630M,E5-2640,8265.030,8472.924,8543.215,8560.117,8657.339,8900.349,8902.260,34
RR,554FLB,E5-2640,544+M,E5-2670v3,12365.830,12446.182,12597.575,12612.538,12790.654,12939.399,12968.120,78
RR,554FLB,E5-2640,650M,E5-2640,8391.870,8516.924,8672.000,8701.399,8977.302,9100.178,9130.380,89
RR,554FLB,E5-2640,554M,E5-2640,6859.680,8390.240,8481.835,8484.884,8649.290,8850.360,8976.960,226
^ permalink raw reply
* Re: [RFC PATCH net-next 2/2] sfc: report 4-tuple UDP hashing to ethtool, if it's enabled
From: Edward Cree @ 2016-09-28 14:56 UTC (permalink / raw)
To: David Laight, linux-net-drivers@solarflare.com,
netdev@vger.kernel.org, davem@davemloft.net
Cc: bkenward@solarflare.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB010CF9F@AcuExch.aculab.com>
On 28/09/16 10:12, David Laight wrote:
> If you invert the above and add a goto...
> if (!efx->rx_hash_udp_4tuple)
> goto set_ip;
I don't mind gotos...
>> case SCTP_V4_FLOW:
>> case AH_ESP_V4_FLOW:
>> case IPV4_FLOW:
> set_ip:
...but this adds a label where we effectively already have one.
I wish C allowed goto case labels.
> It might look better.
> David
It just bugs me that it would have this unnecessary goto and label.
Alternate ways to maybe make it look better, or not:
* Remove the /* else fall further */ comment, does this make the
indentation more or less confusing?
* Include braces on the if, even though there's only one statement
inside.
Also, how strong are people's reaction to this? If it's just "I
personally wouldn't do it that way", then I'm tempted to go ahead
anyway. But if it's "NAK NAK NAK burn the heretic", that's
another matter.
-Ed
^ permalink raw reply
* Re: [PATCH nf-next v3 1/2] netfilter: Fix potential null pointer dereference
From: Aaron Conole @ 2016-09-28 14:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, netdev, Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <1475069735.28155.102.camel@edumazet-glaptop3.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> On Wed, 2016-09-28 at 09:12 -0400, Aaron Conole wrote:
>> It's possible for nf_hook_entry_head to return NULL. If two
>> nf_unregister_net_hook calls happen simultaneously with a single hook
>> entry in the list, both will enter the nf_hook_mutex critical section.
>> The first will successfully delete the head, but the second will see
>> this NULL pointer and attempt to dereference.
>>
>> This fix ensures that no null pointer dereference could occur when such
>> a condition happens.
>>
>> Signed-off-by: Aaron Conole <aconole@bytheb.org>
>> ---
>> net/netfilter/core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
>> index 360c63d..e58e420 100644
>> --- a/net/netfilter/core.c
>> +++ b/net/netfilter/core.c
>> @@ -160,7 +160,7 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
>>
>> mutex_lock(&nf_hook_mutex);
>> hooks_entry = nf_hook_entry_head(net, reg);
>> - if (hooks_entry->orig_ops == reg) {
>> + if (hooks_entry && hooks_entry->orig_ops == reg) {
>> nf_set_hooks_head(net, reg,
>> nf_entry_dereference(hooks_entry->next));
>> goto unlock;
>
> When was the bug added exactly ?
Sunday, on the nf-next tree.
> For all bug fixes, you need to add a Fixes: tag.
>
> Like :
>
> Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
I would but it's in nf-next tree, and I'm not sure how pulls go. If
they are done via patch imports, then the sha sums will be wrong and the
commit message will be misleading. If the sums are preserved, then I
can resubmit with this information.
> So that 100 different people in stable teams do not have to do the
> archeology themselves ...
>
> Thanks.
Thanks for the review, Eric.
^ permalink raw reply
* Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets
From: Cyrill Gorcunov @ 2016-09-28 14:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jamal Hadi Salim, David Ahern, netdev, linux-kernel, David Miller,
kuznet, jmorris, yoshfuji, kaber, avagin, stephen
In-Reply-To: <1475070591.28155.103.camel@edumazet-glaptop3.roam.corp.google.com>
On Wed, Sep 28, 2016 at 06:49:51AM -0700, Eric Dumazet wrote:
> >
> > 4.7 is pretty widespread, so I've to think...
>
> Sorry, 4.4.7 it was
>
> https://www.mail-archive.com/netdev@vger.kernel.org/msg128714.html
Ah, thanks for info!
^ permalink raw reply
* [PATCH net-next v5] bpf: allow access into map value arrays
From: Josef Bacik @ 2016-09-28 14:54 UTC (permalink / raw)
To: davem, daniel, tgraf, netdev, kernel-team, u9012063
Suppose you have a map array value that is something like this
struct foo {
unsigned iter;
int array[SOME_CONSTANT];
};
You can easily insert this into an array, but you cannot modify the contents of
foo->array[] after the fact. This is because we have no way to verify we won't
go off the end of the array at verification time. This patch provides a start
for this work. We accomplish this by keeping track of a minimum and maximum
value a register could be while we're checking the code. Then at the time we
try to do an access into a MAP_VALUE we verify that the maximum offset into that
region is a valid access into that memory region. So in practice, code such as
this
unsigned index = 0;
if (foo->iter >= SOME_CONSTANT)
foo->iter = index;
else
index = foo->iter++;
foo->array[index] = bar;
would be allowed, as we can verify that index will always be between 0 and
SOME_CONSTANT-1. If you wish to use signed values you'll have to have an extra
check to make sure the index isn't less than 0, or do something like index %=
SOME_CONSTANT.
Signed-off-by: Josef Bacik <jbacik@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
V4->V5:
-rebased onto net-next.
-dealt with adding map values to other map values, made it so we only adjust
min/max if we are messing with CONST or INV variables, otherwise we set it to
RANGE_MIN/RANGE_MAX.
-added a new testcase for the adding map values to map values case.
-moved a reset of the register values until after the register validation checks
for BPF_LD
V3->V4:
-fixed the alignment check to deal with MAP_VALUE_ADJ properly.
-updated the error messages for invalid ranges to be more clear.
-fixed test_verifier.c to work with the updated error messages.
-fixed a couple of typos.
V2->V3:
-added testcases to test_verifier.c
-fixed a problem where we wouldn't adjust min/max values if the val == *_RANGE
values, which would cause us to think any access to the map was a-ok.
-Handle the case where the CONST_IMM register is the dst_reg instead of the
src_reg.
-Deal with overflows from the jmp handling.
-Fixed the states_equal logic to make more sense.
V1->V2:
-Instead of using (u64)-1 as the limit which could overflow in some cases
and give us weird behavior, make the limit 1gib so that any math doesn't
overflow and it's still outside of the realm of reasonable offsets.
include/linux/bpf.h | 7 +
include/linux/bpf_verifier.h | 12 ++
kernel/bpf/verifier.c | 329 ++++++++++++++++++++++++++++++++++++++++---
samples/bpf/libbpf.h | 8 ++
samples/bpf/test_verifier.c | 243 +++++++++++++++++++++++++++++++-
5 files changed, 577 insertions(+), 22 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5691fdc..c201017 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -139,6 +139,13 @@ enum bpf_reg_type {
*/
PTR_TO_PACKET,
PTR_TO_PACKET_END, /* skb->data + headlen */
+
+ /* PTR_TO_MAP_VALUE_ADJ is used for doing pointer math inside of a map
+ * elem value. We only allow this if we can statically verify that
+ * access from this register are going to fall within the size of the
+ * map element.
+ */
+ PTR_TO_MAP_VALUE_ADJ,
};
struct bpf_prog;
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index c5cb661..7035b99 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -10,8 +10,19 @@
#include <linux/bpf.h> /* for enum bpf_reg_type */
#include <linux/filter.h> /* for MAX_BPF_STACK */
+ /* Just some arbitrary values so we can safely do math without overflowing and
+ * are obviously wrong for any sort of memory access.
+ */
+#define BPF_REGISTER_MAX_RANGE (1024 * 1024 * 1024)
+#define BPF_REGISTER_MIN_RANGE -(1024 * 1024 * 1024)
+
struct bpf_reg_state {
enum bpf_reg_type type;
+ /*
+ * Used to determine if any memory access using this register will
+ * result in a bad access.
+ */
+ u64 min_value, max_value;
union {
/* valid when type == CONST_IMM | PTR_TO_STACK | UNKNOWN_VALUE */
s64 imm;
@@ -81,6 +92,7 @@ struct bpf_verifier_env {
u32 id_gen; /* used to generate unique reg IDs */
bool allow_ptr_leaks;
bool seen_direct_write;
+ bool varlen_map_value_access;
struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
};
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7ada315..99a7e5b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -182,6 +182,7 @@ static const char * const reg_type_str[] = {
[CONST_PTR_TO_MAP] = "map_ptr",
[PTR_TO_MAP_VALUE] = "map_value",
[PTR_TO_MAP_VALUE_OR_NULL] = "map_value_or_null",
+ [PTR_TO_MAP_VALUE_ADJ] = "map_value_adj",
[FRAME_PTR] = "fp",
[PTR_TO_STACK] = "fp",
[CONST_IMM] = "imm",
@@ -209,10 +210,17 @@ static void print_verifier_state(struct bpf_verifier_state *state)
else if (t == UNKNOWN_VALUE && reg->imm)
verbose("%lld", reg->imm);
else if (t == CONST_PTR_TO_MAP || t == PTR_TO_MAP_VALUE ||
- t == PTR_TO_MAP_VALUE_OR_NULL)
+ t == PTR_TO_MAP_VALUE_OR_NULL ||
+ t == PTR_TO_MAP_VALUE_ADJ)
verbose("(ks=%d,vs=%d)",
reg->map_ptr->key_size,
reg->map_ptr->value_size);
+ if (reg->min_value != BPF_REGISTER_MIN_RANGE)
+ verbose(",min_value=%llu",
+ (unsigned long long)reg->min_value);
+ if (reg->max_value != BPF_REGISTER_MAX_RANGE)
+ verbose(",max_value=%llu",
+ (unsigned long long)reg->max_value);
}
for (i = 0; i < MAX_BPF_STACK; i += BPF_REG_SIZE) {
if (state->stack_slot_type[i] == STACK_SPILL)
@@ -424,6 +432,8 @@ static void init_reg_state(struct bpf_reg_state *regs)
for (i = 0; i < MAX_BPF_REG; i++) {
regs[i].type = NOT_INIT;
regs[i].imm = 0;
+ regs[i].min_value = BPF_REGISTER_MIN_RANGE;
+ regs[i].max_value = BPF_REGISTER_MAX_RANGE;
}
/* frame pointer */
@@ -440,6 +450,12 @@ static void mark_reg_unknown_value(struct bpf_reg_state *regs, u32 regno)
regs[regno].imm = 0;
}
+static void reset_reg_range_values(struct bpf_reg_state *regs, u32 regno)
+{
+ regs[regno].min_value = BPF_REGISTER_MIN_RANGE;
+ regs[regno].max_value = BPF_REGISTER_MAX_RANGE;
+}
+
enum reg_arg_type {
SRC_OP, /* register is used as source operand */
DST_OP, /* register is used as destination operand */
@@ -665,7 +681,7 @@ static bool is_pointer_value(struct bpf_verifier_env *env, int regno)
static int check_ptr_alignment(struct bpf_verifier_env *env,
struct bpf_reg_state *reg, int off, int size)
{
- if (reg->type != PTR_TO_PACKET) {
+ if (reg->type != PTR_TO_PACKET && reg->type != PTR_TO_MAP_VALUE_ADJ) {
if (off % size != 0) {
verbose("misaligned access off %d size %d\n",
off, size);
@@ -675,16 +691,6 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
}
}
- switch (env->prog->type) {
- case BPF_PROG_TYPE_SCHED_CLS:
- case BPF_PROG_TYPE_SCHED_ACT:
- case BPF_PROG_TYPE_XDP:
- break;
- default:
- verbose("verifier is misconfigured\n");
- return -EACCES;
- }
-
if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
/* misaligned access to packet is ok on x86,arm,arm64 */
return 0;
@@ -695,7 +701,8 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
}
/* skb->data is NET_IP_ALIGN-ed */
- if ((NET_IP_ALIGN + reg->off + off) % size != 0) {
+ if (reg->type == PTR_TO_PACKET &&
+ (NET_IP_ALIGN + reg->off + off) % size != 0) {
verbose("misaligned packet access off %d+%d+%d size %d\n",
NET_IP_ALIGN, reg->off, off, size);
return -EACCES;
@@ -728,12 +735,52 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off,
if (err)
return err;
- if (reg->type == PTR_TO_MAP_VALUE) {
+ if (reg->type == PTR_TO_MAP_VALUE ||
+ reg->type == PTR_TO_MAP_VALUE_ADJ) {
if (t == BPF_WRITE && value_regno >= 0 &&
is_pointer_value(env, value_regno)) {
verbose("R%d leaks addr into map\n", value_regno);
return -EACCES;
}
+
+ /* If we adjusted the register to this map value at all then we
+ * need to change off and size to min_value and max_value
+ * respectively to make sure our theoretical access will be
+ * safe.
+ */
+ if (reg->type == PTR_TO_MAP_VALUE_ADJ) {
+ if (log_level)
+ print_verifier_state(state);
+ env->varlen_map_value_access = true;
+ /* The minimum value is only important with signed
+ * comparisons where we can't assume the floor of a
+ * value is 0. If we are using signed variables for our
+ * index'es we need to make sure that whatever we use
+ * will have a set floor within our range.
+ */
+ if ((s64)reg->min_value < 0) {
+ verbose("R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
+ regno);
+ return -EACCES;
+ }
+ err = check_map_access(env, regno, reg->min_value + off,
+ size);
+ if (err) {
+ verbose("R%d min value is outside of the array range\n",
+ regno);
+ return err;
+ }
+
+ /* If we haven't set a max value then we need to bail
+ * since we can't be sure we won't do bad things.
+ */
+ if (reg->max_value == BPF_REGISTER_MAX_RANGE) {
+ verbose("R%d unbounded memory access, make sure to bounds check any array access into a map\n",
+ regno);
+ return -EACCES;
+ }
+ off += reg->max_value;
+ }
err = check_map_access(env, regno, off, size);
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown_value(state->regs, value_regno);
@@ -1195,6 +1242,7 @@ static int check_call(struct bpf_verifier_env *env, int func_id)
regs[BPF_REG_0].type = NOT_INIT;
} else if (fn->ret_type == RET_PTR_TO_MAP_VALUE_OR_NULL) {
regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
+ regs[BPF_REG_0].max_value = regs[BPF_REG_0].min_value = 0;
/* remember map_ptr, so that check_map_access()
* can check 'value_size' boundary of memory access
* to map element returned from bpf_map_lookup_elem()
@@ -1416,6 +1464,106 @@ static int evaluate_reg_imm_alu(struct bpf_verifier_env *env,
return 0;
}
+static void check_reg_overflow(struct bpf_reg_state *reg)
+{
+ if (reg->max_value > BPF_REGISTER_MAX_RANGE)
+ reg->max_value = BPF_REGISTER_MAX_RANGE;
+ if ((s64)reg->min_value < BPF_REGISTER_MIN_RANGE)
+ reg->min_value = BPF_REGISTER_MIN_RANGE;
+}
+
+static void adjust_reg_min_max_vals(struct bpf_verifier_env *env,
+ struct bpf_insn *insn)
+{
+ struct bpf_reg_state *regs = env->cur_state.regs, *dst_reg;
+ u64 min_val = BPF_REGISTER_MIN_RANGE, max_val = BPF_REGISTER_MAX_RANGE;
+ bool min_set = false, max_set = false;
+ u8 opcode = BPF_OP(insn->code);
+
+ dst_reg = ®s[insn->dst_reg];
+ if (BPF_SRC(insn->code) == BPF_X) {
+ check_reg_overflow(®s[insn->src_reg]);
+ min_val = regs[insn->src_reg].min_value;
+ max_val = regs[insn->src_reg].max_value;
+
+ /* If the source register is a random pointer then the
+ * min_value/max_value values represent the range of the known
+ * accesses into that value, not the actual min/max value of the
+ * register itself. In this case we have to reset the reg range
+ * values so we know it is not safe to look at.
+ */
+ if (regs[insn->src_reg].type != CONST_IMM &&
+ regs[insn->src_reg].type != UNKNOWN_VALUE) {
+ min_val = BPF_REGISTER_MIN_RANGE;
+ max_val = BPF_REGISTER_MAX_RANGE;
+ }
+ } else if (insn->imm < BPF_REGISTER_MAX_RANGE &&
+ (s64)insn->imm > BPF_REGISTER_MIN_RANGE) {
+ min_val = max_val = insn->imm;
+ min_set = max_set = true;
+ }
+
+ /* We don't know anything about what was done to this register, mark it
+ * as unknown.
+ */
+ if (min_val == BPF_REGISTER_MIN_RANGE &&
+ max_val == BPF_REGISTER_MAX_RANGE) {
+ reset_reg_range_values(regs, insn->dst_reg);
+ return;
+ }
+
+ switch (opcode) {
+ case BPF_ADD:
+ dst_reg->min_value += min_val;
+ dst_reg->max_value += max_val;
+ break;
+ case BPF_SUB:
+ dst_reg->min_value -= min_val;
+ dst_reg->max_value -= max_val;
+ break;
+ case BPF_MUL:
+ dst_reg->min_value *= min_val;
+ dst_reg->max_value *= max_val;
+ break;
+ case BPF_AND:
+ /* & is special since it could end up with 0 bits set. */
+ dst_reg->min_value &= min_val;
+ dst_reg->max_value = max_val;
+ break;
+ case BPF_LSH:
+ /* Gotta have special overflow logic here, if we're shifting
+ * more than MAX_RANGE then just assume we have an invalid
+ * range.
+ */
+ if (min_val > ilog2(BPF_REGISTER_MAX_RANGE))
+ dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
+ else
+ dst_reg->min_value <<= min_val;
+
+ if (max_val > ilog2(BPF_REGISTER_MAX_RANGE))
+ dst_reg->max_value = BPF_REGISTER_MAX_RANGE;
+ else
+ dst_reg->max_value <<= max_val;
+ break;
+ case BPF_RSH:
+ dst_reg->min_value >>= min_val;
+ dst_reg->max_value >>= max_val;
+ break;
+ case BPF_MOD:
+ /* % is special since it is an unsigned modulus, so the floor
+ * will always be 0.
+ */
+ dst_reg->min_value = 0;
+ dst_reg->max_value = max_val - 1;
+ break;
+ default:
+ reset_reg_range_values(regs, insn->dst_reg);
+ break;
+ }
+
+ check_reg_overflow(dst_reg);
+}
+
/* check validity of 32-bit and 64-bit arithmetic operations */
static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
{
@@ -1479,6 +1627,11 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
if (err)
return err;
+ /* we are setting our register to something new, we need to
+ * reset its range values.
+ */
+ reset_reg_range_values(regs, insn->dst_reg);
+
if (BPF_SRC(insn->code) == BPF_X) {
if (BPF_CLASS(insn->code) == BPF_ALU64) {
/* case: R1 = R2
@@ -1500,6 +1653,8 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
*/
regs[insn->dst_reg].type = CONST_IMM;
regs[insn->dst_reg].imm = insn->imm;
+ regs[insn->dst_reg].max_value = insn->imm;
+ regs[insn->dst_reg].min_value = insn->imm;
}
} else if (opcode > BPF_END) {
@@ -1552,6 +1707,9 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
dst_reg = ®s[insn->dst_reg];
+ /* first we want to adjust our ranges. */
+ adjust_reg_min_max_vals(env, insn);
+
/* pattern match 'bpf_add Rx, imm' instruction */
if (opcode == BPF_ADD && BPF_CLASS(insn->code) == BPF_ALU64 &&
dst_reg->type == FRAME_PTR && BPF_SRC(insn->code) == BPF_K) {
@@ -1586,8 +1744,17 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
return -EACCES;
}
- /* mark dest operand */
- mark_reg_unknown_value(regs, insn->dst_reg);
+ /* If we did pointer math on a map value then just set it to our
+ * PTR_TO_MAP_VALUE_ADJ type so we can deal with any stores or
+ * loads to this register appropriately, otherwise just mark the
+ * register as unknown.
+ */
+ if (env->allow_ptr_leaks &&
+ (dst_reg->type == PTR_TO_MAP_VALUE ||
+ dst_reg->type == PTR_TO_MAP_VALUE_ADJ))
+ dst_reg->type = PTR_TO_MAP_VALUE_ADJ;
+ else
+ mark_reg_unknown_value(regs, insn->dst_reg);
}
return 0;
@@ -1642,6 +1809,104 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *state,
}
}
+/* Adjusts the register min/max values in the case that the dst_reg is the
+ * variable register that we are working on, and src_reg is a constant or we're
+ * simply doing a BPF_K check.
+ */
+static void reg_set_min_max(struct bpf_reg_state *true_reg,
+ struct bpf_reg_state *false_reg, u64 val,
+ u8 opcode)
+{
+ switch (opcode) {
+ case BPF_JEQ:
+ /* If this is false then we know nothing Jon Snow, but if it is
+ * true then we know for sure.
+ */
+ true_reg->max_value = true_reg->min_value = val;
+ break;
+ case BPF_JNE:
+ /* If this is true we know nothing Jon Snow, but if it is false
+ * we know the value for sure;
+ */
+ false_reg->max_value = false_reg->min_value = val;
+ break;
+ case BPF_JGT:
+ /* Unsigned comparison, the minimum value is 0. */
+ false_reg->min_value = 0;
+ case BPF_JSGT:
+ /* If this is false then we know the maximum val is val,
+ * otherwise we know the min val is val+1.
+ */
+ false_reg->max_value = val;
+ true_reg->min_value = val + 1;
+ break;
+ case BPF_JGE:
+ /* Unsigned comparison, the minimum value is 0. */
+ false_reg->min_value = 0;
+ case BPF_JSGE:
+ /* If this is false then we know the maximum value is val - 1,
+ * otherwise we know the mimimum value is val.
+ */
+ false_reg->max_value = val - 1;
+ true_reg->min_value = val;
+ break;
+ default:
+ break;
+ }
+
+ check_reg_overflow(false_reg);
+ check_reg_overflow(true_reg);
+}
+
+/* Same as above, but for the case that dst_reg is a CONST_IMM reg and src_reg
+ * is the variable reg.
+ */
+static void reg_set_min_max_inv(struct bpf_reg_state *true_reg,
+ struct bpf_reg_state *false_reg, u64 val,
+ u8 opcode)
+{
+ switch (opcode) {
+ case BPF_JEQ:
+ /* If this is false then we know nothing Jon Snow, but if it is
+ * true then we know for sure.
+ */
+ true_reg->max_value = true_reg->min_value = val;
+ break;
+ case BPF_JNE:
+ /* If this is true we know nothing Jon Snow, but if it is false
+ * we know the value for sure;
+ */
+ false_reg->max_value = false_reg->min_value = val;
+ break;
+ case BPF_JGT:
+ /* Unsigned comparison, the minimum value is 0. */
+ true_reg->min_value = 0;
+ case BPF_JSGT:
+ /*
+ * If this is false, then the val is <= the register, if it is
+ * true the register <= to the val.
+ */
+ false_reg->min_value = val;
+ true_reg->max_value = val - 1;
+ break;
+ case BPF_JGE:
+ /* Unsigned comparison, the minimum value is 0. */
+ true_reg->min_value = 0;
+ case BPF_JSGE:
+ /* If this is false then constant < register, if it is true then
+ * the register < constant.
+ */
+ false_reg->min_value = val + 1;
+ true_reg->max_value = val;
+ break;
+ default:
+ break;
+ }
+
+ check_reg_overflow(false_reg);
+ check_reg_overflow(true_reg);
+}
+
static int check_cond_jmp_op(struct bpf_verifier_env *env,
struct bpf_insn *insn, int *insn_idx)
{
@@ -1708,6 +1973,23 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
if (!other_branch)
return -EFAULT;
+ /* detect if we are comparing against a constant value so we can adjust
+ * our min/max values for our dst register.
+ */
+ if (BPF_SRC(insn->code) == BPF_X) {
+ if (regs[insn->src_reg].type == CONST_IMM)
+ reg_set_min_max(&other_branch->regs[insn->dst_reg],
+ dst_reg, regs[insn->src_reg].imm,
+ opcode);
+ else if (dst_reg->type == CONST_IMM)
+ reg_set_min_max_inv(&other_branch->regs[insn->src_reg],
+ ®s[insn->src_reg], dst_reg->imm,
+ opcode);
+ } else {
+ reg_set_min_max(&other_branch->regs[insn->dst_reg],
+ dst_reg, insn->imm, opcode);
+ }
+
/* detect if R == 0 where R is returned from bpf_map_lookup_elem() */
if (BPF_SRC(insn->code) == BPF_K &&
insn->imm == 0 && (opcode == BPF_JEQ || opcode == BPF_JNE) &&
@@ -2144,7 +2426,8 @@ static bool compare_ptrs_to_packet(struct bpf_reg_state *old,
* whereas register type in current state is meaningful, it means that
* the current state will reach 'bpf_exit' instruction safely
*/
-static bool states_equal(struct bpf_verifier_state *old,
+static bool states_equal(struct bpf_verifier_env *env,
+ struct bpf_verifier_state *old,
struct bpf_verifier_state *cur)
{
struct bpf_reg_state *rold, *rcur;
@@ -2157,6 +2440,13 @@ static bool states_equal(struct bpf_verifier_state *old,
if (memcmp(rold, rcur, sizeof(*rold)) == 0)
continue;
+ /* If the ranges were not the same, but everything else was and
+ * we didn't do a variable access into a map then we are a-ok.
+ */
+ if (!env->varlen_map_value_access &&
+ rold->type == rcur->type && rold->imm == rcur->imm)
+ continue;
+
if (rold->type == NOT_INIT ||
(rold->type == UNKNOWN_VALUE && rcur->type != NOT_INIT))
continue;
@@ -2213,7 +2503,7 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
return 0;
while (sl != STATE_LIST_MARK) {
- if (states_equal(&sl->state, &env->cur_state))
+ if (states_equal(env, &sl->state, &env->cur_state))
/* reached equivalent register/stack state,
* prune the search
*/
@@ -2259,6 +2549,7 @@ static int do_check(struct bpf_verifier_env *env)
init_reg_state(regs);
insn_idx = 0;
+ env->varlen_map_value_access = false;
for (;;) {
struct bpf_insn *insn;
u8 class;
@@ -2339,6 +2630,7 @@ static int do_check(struct bpf_verifier_env *env)
if (err)
return err;
+ reset_reg_range_values(regs, insn->dst_reg);
if (BPF_SIZE(insn->code) != BPF_W &&
BPF_SIZE(insn->code) != BPF_DW) {
insn_idx++;
@@ -2509,6 +2801,7 @@ process_bpf_exit:
verbose("invalid BPF_LD mode\n");
return -EINVAL;
}
+ reset_reg_range_values(regs, insn->dst_reg);
} else {
verbose("unknown insn class %d\n", class);
return -EINVAL;
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index 364582b..ac6edb6 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -85,6 +85,14 @@ extern char bpf_log_buf[LOG_BUF_SIZE];
.off = 0, \
.imm = IMM })
+#define BPF_MOV32_IMM(DST, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_MOV | BPF_K, \
+ .dst_reg = DST, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = IMM })
+
/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
#define BPF_LD_IMM64(DST, IMM) \
BPF_LD_IMM64_RAW(DST, 0, IMM)
diff --git a/samples/bpf/test_verifier.c b/samples/bpf/test_verifier.c
index ac590d4..369ffaa 100644
--- a/samples/bpf/test_verifier.c
+++ b/samples/bpf/test_verifier.c
@@ -29,6 +29,7 @@ struct bpf_test {
struct bpf_insn insns[MAX_INSNS];
int fixup[MAX_FIXUPS];
int prog_array_fixup[MAX_FIXUPS];
+ int test_val_map_fixup[MAX_FIXUPS];
const char *errstr;
const char *errstr_unpriv;
enum {
@@ -39,6 +40,19 @@ struct bpf_test {
enum bpf_prog_type prog_type;
};
+/* Note we want this to be 64 bit aligned so that the end of our array is
+ * actually the end of the structure.
+ */
+#define MAX_ENTRIES 11
+struct test_val {
+ unsigned index;
+ int foo[MAX_ENTRIES];
+};
+
+struct other_val {
+ unsigned int action[32];
+};
+
static struct bpf_test tests[] = {
{
"add+sub+mul",
@@ -2163,6 +2177,212 @@ static struct bpf_test tests[] = {
.errstr = "invalid access to packet",
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
},
+ {
+ "valid map access into an array with a constant",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr_unpriv = "R0 leaks addr",
+ .result_unpriv = REJECT,
+ .result = ACCEPT,
+ },
+ {
+ "valid map access into an array with a register",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+ BPF_MOV64_IMM(BPF_REG_1, 4),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr_unpriv = "R0 leaks addr",
+ .result_unpriv = REJECT,
+ .result = ACCEPT,
+ },
+ {
+ "valid map access into an array with a variable",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 5),
+ BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
+ BPF_JMP_IMM(BPF_JGE, BPF_REG_1, MAX_ENTRIES, 3),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr_unpriv = "R0 leaks addr",
+ .result_unpriv = REJECT,
+ .result = ACCEPT,
+ },
+ {
+ "valid map access into an array with a signed variable",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 9),
+ BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
+ BPF_JMP_IMM(BPF_JSGT, BPF_REG_1, 0xffffffff, 1),
+ BPF_MOV32_IMM(BPF_REG_1, 0),
+ BPF_MOV32_IMM(BPF_REG_2, MAX_ENTRIES),
+ BPF_JMP_REG(BPF_JSGT, BPF_REG_2, BPF_REG_1, 1),
+ BPF_MOV32_IMM(BPF_REG_1, 0),
+ BPF_ALU32_IMM(BPF_LSH, BPF_REG_1, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr_unpriv = "R0 leaks addr",
+ .result_unpriv = REJECT,
+ .result = ACCEPT,
+ },
+ {
+ "invalid map access into an array with a constant",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, (MAX_ENTRIES + 1) << 2,
+ offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr = "invalid access to map value, value_size=48 off=48 size=8",
+ .result = REJECT,
+ },
+ {
+ "invalid map access into an array with a register",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+ BPF_MOV64_IMM(BPF_REG_1, MAX_ENTRIES + 1),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr = "R0 min value is outside of the array range",
+ .result = REJECT,
+ },
+ {
+ "invalid map access into an array with a variable",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+ BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
+ .result = REJECT,
+ },
+ {
+ "invalid map access into an array with no floor check",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 7),
+ BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
+ BPF_MOV32_IMM(BPF_REG_2, MAX_ENTRIES),
+ BPF_JMP_REG(BPF_JSGT, BPF_REG_2, BPF_REG_1, 1),
+ BPF_MOV32_IMM(BPF_REG_1, 0),
+ BPF_ALU32_IMM(BPF_LSH, BPF_REG_1, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
+ .result = REJECT,
+ },
+ {
+ "invalid map access into an array with a invalid max check",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 7),
+ BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
+ BPF_MOV32_IMM(BPF_REG_2, MAX_ENTRIES + 1),
+ BPF_JMP_REG(BPF_JGT, BPF_REG_2, BPF_REG_1, 1),
+ BPF_MOV32_IMM(BPF_REG_1, 0),
+ BPF_ALU32_IMM(BPF_LSH, BPF_REG_1, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3},
+ .errstr = "invalid access to map value, value_size=48 off=44 size=8",
+ .result = REJECT,
+ },
+ {
+ "invalid map access into an array with a invalid max check",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 10),
+ BPF_MOV64_REG(BPF_REG_8, BPF_REG_0),
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_8),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, offsetof(struct test_val, foo)),
+ BPF_EXIT_INSN(),
+ },
+ .test_val_map_fixup = {3, 11},
+ .errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
+ .result = REJECT,
+ },
};
static int probe_filter_length(struct bpf_insn *fp)
@@ -2176,12 +2396,12 @@ static int probe_filter_length(struct bpf_insn *fp)
return len + 1;
}
-static int create_map(void)
+static int create_map(size_t val_size, int num)
{
int map_fd;
map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
- sizeof(long long), sizeof(long long), 1024, 0);
+ sizeof(long long), val_size, num, 0);
if (map_fd < 0)
printf("failed to create map '%s'\n", strerror(errno));
@@ -2211,12 +2431,13 @@ static int test(void)
int prog_len = probe_filter_length(prog);
int *fixup = tests[i].fixup;
int *prog_array_fixup = tests[i].prog_array_fixup;
+ int *test_val_map_fixup = tests[i].test_val_map_fixup;
int expected_result;
const char *expected_errstr;
- int map_fd = -1, prog_array_fd = -1;
+ int map_fd = -1, prog_array_fd = -1, test_val_map_fd = -1;
if (*fixup) {
- map_fd = create_map();
+ map_fd = create_map(sizeof(long long), 1024);
do {
prog[*fixup].imm = map_fd;
@@ -2231,6 +2452,18 @@ static int test(void)
prog_array_fixup++;
} while (*prog_array_fixup);
}
+ if (*test_val_map_fixup) {
+ /* Unprivileged can't create a hash map.*/
+ if (unpriv)
+ continue;
+ test_val_map_fd = create_map(sizeof(struct test_val),
+ 256);
+ do {
+ prog[*test_val_map_fixup].imm = test_val_map_fd;
+ test_val_map_fixup++;
+ } while (*test_val_map_fixup);
+ }
+
printf("#%d %s ", i, tests[i].descr);
prog_fd = bpf_prog_load(prog_type ?: BPF_PROG_TYPE_SOCKET_FILTER,
@@ -2277,6 +2510,8 @@ fail:
close(map_fd);
if (prog_array_fd >= 0)
close(prog_array_fd);
+ if (test_val_map_fd >= 0)
+ close(test_val_map_fd);
close(prog_fd);
}
--
2.7.4
^ permalink raw reply related
* [PATCH v8 7/8] thunderbolt: Networking doc
From: Amir Levy @ 2016-09-28 14:44 UTC (permalink / raw)
To: gregkh
Cc: andreas.noever, bhelgaas, corbet, linux-kernel, linux-pci, netdev,
linux-doc, mario_limonciello, thunderbolt-linux, mika.westerberg,
tomas.winkler, xiong.y.zhang, Amir Levy
In-Reply-To: <1475073870-2126-1-git-send-email-amir.jer.levy@intel.com>
Adding Thunderbolt(TM) networking documentation.
Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
---
Documentation/00-INDEX | 2 +
Documentation/thunderbolt/networking.txt | 132 +++++++++++++++++++++++++++++++
2 files changed, 134 insertions(+)
create mode 100644 Documentation/thunderbolt/networking.txt
diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index cb9a6c6..a448ba1 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -439,6 +439,8 @@ this_cpu_ops.txt
- List rationale behind and the way to use this_cpu operations.
thermal/
- directory with information on managing thermal issues (CPU/temp)
+thunderbolt/
+ - directory with info regarding Thunderbolt.
trace/
- directory with info on tracing technologies within linux
unaligned-memory-access.txt
diff --git a/Documentation/thunderbolt/networking.txt b/Documentation/thunderbolt/networking.txt
new file mode 100644
index 0000000..88d1c12
--- /dev/null
+++ b/Documentation/thunderbolt/networking.txt
@@ -0,0 +1,132 @@
+Intel Thunderbolt(TM) Networking driver
+=======================================
+
+Copyright(c) 2013 - 2016 Intel Corporation.
+
+Contact Information:
+Intel Thunderbolt mailing list <thunderbolt-software@lists.01.org>
+Edited by Amir Levy <amir.jer.levy@intel.com>
+
+Overview
+========
+
+* The Thunderbolt Networking driver enables peer to peer networking on non-Apple
+ platforms running Linux.
+
+* The driver creates a virtual Ethernet device that enables computer to computer
+ communication over the Thunderbolt cable.
+
+* Using Thunderbolt Networking you can perform high speed file transfers between
+ computers, perform PC migrations and/or set up small workgroups with shared
+ storage without compromising any other Thunderbolt functionality.
+
+* The driver is located in drivers/thunderbolt/icm.
+
+* This driver will function only on non-Apple platforms with firmware based
+ Thunderbolt controllers that support Thunderbolt Networking.
+
+ +----------------+ +----------------+
+ |Host 1 | |Host 2 |
+ | | | |
+ | +-------+ | | +-------+ |
+ | |Network| | | |Network| |
+ | |Stack | | | |Stack | |
+ | +-------+ | | +-------+ |
+ | ^ | | ^ |
+ | | | | | |
+ | v | | v |
+ | +-----------+ | | +-----------+ |
+ | |Thunderbolt| | | |Thunderbolt| |
+ | |Networking | | | |Networking | |
+ | |Driver | | | |Driver | |
+ | +-----------+ | | +-----------+ |
+ | ^ | | ^ |
+ | | | | | |
+ | v | | v |
+ | +-----------+ | | +-----------+ |
+ | |Thunderbolt| | | |Thunderbolt| |
+ | |Controller |<-+------------+->|Controller | |
+ | |with ICM | | | |with ICM | |
+ | |enabled | | | |enabled | |
+ | +-----------+ | | +-----------+ |
+ +----------------+ +----------------+
+
+Files
+=====
+
+The following files are located in the drivers/thunderbolt/icm directory:
+
+- icm_nhi.c/h: These files allow communication with the firmware (Intel
+ Connection Manager) based controller. They also create an interface for
+ netlink communication with a user space daemon.
+
+- net.c/net.h: These files implement the 'eth' interface for the
+ Thunderbolt(TM) Networking.
+
+Interface to User Space
+=======================
+
+The interface to the user space module is implemented through a Generic Netlink.
+This is the communications protocol between the Thunderbolt driver and the user
+space application.
+
+Note that this interface mediates user space communication with ICM.
+(Existing Linux tools can be used to configure the network interface.)
+
+The Thunderbolt Daemon utilizes this interface to communicate with the driver.
+To be accessed by the user space module, both kernel and user space modules
+have to register with the same GENL_NAME.
+For the purpose of the Thunderbolt Network driver, "thunderbolt" is used.
+The registration is done at driver initialization time for all instances
+of the Thunderbolt controllers. The communication is carried through pre-defined
+Thunderbolt messages. Each specific message has a callback function that is
+called when the related message is received.
+
+Message Definitions:
+* NHI_CMD_UNSPEC: Not used.
+* NHI_CMD_SUBSCRIBE: Subscription request from daemon to driver to open the
+ communication channel.
+* NHI_CMD_UNSUBSCRIBE: Request from daemon to driver to unsubscribe and
+ to close communication channel.
+* NHI_CMD_QUERY_INFORMATION: Request information from the driver such as
+ driver version, FW version offset, number of ports in the controller
+ and DMA port.
+* NHI_CMD_MSG_TO_ICM: Message from user space module to FW.
+* NHI_CMD_MSG_FROM_ICM: Response from FW to user space module.
+* NHI_CMD_MAILBOX: Message that uses mailbox mechanism such as FW policy
+ changes or disconnect path.
+* NHI_CMD_APPROVE_TBT_NETWORKING: Request from user space module to FW to
+ establish path.
+* NHI_CMD_ICM_IN_SAFE_MODE: Indication that the FW has entered safe mode.
+
+Communication with Intel Connection Manager(ICM) Firmware
+=========================================================
+
+There are several circular buffers in Thunderbolt each using Direct Memory
+Access (DMA).
+
+Communication with ICM utilizes circular buffer ring #0. (The other rings are
+used for peer to peer communication, packet transmission and receiving).
+
+The driver allocates a shared memory that is physically mapped onto the DMA
+physical space at ring #0.
+For the software to communicate with the firmware, the driver sends a command
+in ring #0. The command contains a pre-defined field (PDF) value notifying the
+firmware that the driver is ready. To proceed, the driver must receive the
+appropriate PDF value in response from the firmware.
+
+Once the exchange is completed, messages can be sent to the firmware through
+the driver. Similarly, the firmware can now send notifications about hardware
+and firmware events.
+
+Information
+===========
+
+Mailing list:
+ thunderbolt-software@lists.01.org
+ Register at: https://lists.01.org/mailman/listinfo/thunderbolt-software
+ Archives at: https://lists.01.org/pipermail/thunderbolt-software/
+
+For additional information about Thunderbolt technology visit:
+ https://01.org/thunderbolt-sw
+ https://thunderbolttechnology.net/
--
2.7.4
^ permalink raw reply related
* [PATCH v8 6/8] thunderbolt: Kconfig for Thunderbolt Networking
From: Amir Levy @ 2016-09-28 14:44 UTC (permalink / raw)
To: gregkh
Cc: andreas.noever, bhelgaas, corbet, linux-kernel, linux-pci, netdev,
linux-doc, mario_limonciello, thunderbolt-linux, mika.westerberg,
tomas.winkler, xiong.y.zhang, Amir Levy
In-Reply-To: <1475073870-2126-1-git-send-email-amir.jer.levy@intel.com>
Update to the Kconfig Thunderbolt description to add
Thunderbolt networking as an option.
The menu item "Thunderbolt support" now offers:
"Apple Hardware Support" (existing)
and/or
"Thunderbolt Networking" (new)
You can choose the driver for your platform or build both drivers -
each driver will detect if it can run on the specific platform.
If the Thunderbolt Networking option is chosen, Thunderbolt Networking
will be enabled between Linux non-Apple systems, macOS and
Windows based systems.
Thunderbolt Networking will not affect any other Thunderbolt feature that
was previous available to Linux users on either Apple or
non-Apple platforms.
Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
---
drivers/thunderbolt/Kconfig | 27 +++++++++++++++++++++++----
drivers/thunderbolt/Makefile | 3 ++-
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index c121acc..376e5bb 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -1,13 +1,32 @@
-menuconfig THUNDERBOLT
- tristate "Thunderbolt support for Apple devices"
+config THUNDERBOLT
+ tristate "Thunderbolt support"
depends on PCI
select CRC32
help
- Cactus Ridge Thunderbolt Controller driver
+ Thunderbolt Controller driver
+
+if THUNDERBOLT
+
+config THUNDERBOLT_APPLE
+ tristate "Apple hardware support"
+ help
This driver is required if you want to hotplug Thunderbolt devices on
Apple hardware.
Device chaining is currently not supported.
- To compile this driver a module, choose M here. The module will be
+ To compile this driver as a module, choose M here. The module will be
called thunderbolt.
+
+config THUNDERBOLT_ICM
+ tristate "Thunderbolt Networking"
+ help
+ This driver is required if you want Thunderbolt Networking on
+ non-Apple hardware.
+ It creates a virtual Ethernet device that enables computer to
+ computer communication over a Thunderbolt cable.
+
+ To compile this driver as a module, choose M here. The module will be
+ called thunderbolt_icm.
+
+endif
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 5d1053c..b6aa6a3 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,4 @@
-obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
+obj-${CONFIG_THUNDERBOLT_APPLE} := thunderbolt.o
thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o eeprom.o
+obj-${CONFIG_THUNDERBOLT_ICM} += icm/
--
2.7.4
^ permalink raw reply related
* [PATCH v8 4/8] thunderbolt: Networking state machine
From: Amir Levy @ 2016-09-28 14:44 UTC (permalink / raw)
To: gregkh
Cc: andreas.noever, bhelgaas, corbet, linux-kernel, linux-pci, netdev,
linux-doc, mario_limonciello, thunderbolt-linux, mika.westerberg,
tomas.winkler, xiong.y.zhang, Amir Levy
In-Reply-To: <1475073870-2126-1-git-send-email-amir.jer.levy@intel.com>
This patch builds the peer to peer communication path.
Communication is established by a negotiation process whereby messages are
sent back and forth between the peers until a connection is established.
This includes the Thunderbolt Network driver communication with the second
peer via Intel Connection Manager(ICM) firmware.
+--------------------+ +--------------------+
|Host 1 | |Host 2 |
| | | |
| +-----------+ | | +-----------+ |
| |Thunderbolt| | | |Thunderbolt| |
| |Networking | | | |Networking | |
| |Driver | | | |Driver | |
| +-----------+ | | +-----------+ |
| ^ | | ^ |
| | | | | |
| +------------+---+ | | +------------+---+ |
| |Thunderbolt | | | | |Thunderbolt | | |
| |Controller v | | | |Controller v | |
| | +---+ | | | | +---+ | |
| | |ICM|<-+-+------------+-+-------->|ICM| | |
| | +---+ | | | | +---+ | |
| +----------------+ | | +----------------+ |
+--------------------+ +--------------------+
Note that this patch only establishes the link between the two hosts and
not Network Packet handling - this is dealt with in the next patch.
Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
---
drivers/thunderbolt/icm/Makefile | 2 +-
drivers/thunderbolt/icm/icm_nhi.c | 262 ++++++++++++-
drivers/thunderbolt/icm/net.c | 783 ++++++++++++++++++++++++++++++++++++++
drivers/thunderbolt/icm/net.h | 70 ++++
4 files changed, 1109 insertions(+), 8 deletions(-)
create mode 100644 drivers/thunderbolt/icm/net.c
diff --git a/drivers/thunderbolt/icm/Makefile b/drivers/thunderbolt/icm/Makefile
index f0d0fbb..94a2797 100644
--- a/drivers/thunderbolt/icm/Makefile
+++ b/drivers/thunderbolt/icm/Makefile
@@ -1,2 +1,2 @@
obj-${CONFIG_THUNDERBOLT_ICM} += thunderbolt-icm.o
-thunderbolt-icm-objs := icm_nhi.o
+thunderbolt-icm-objs := icm_nhi.o net.o
diff --git a/drivers/thunderbolt/icm/icm_nhi.c b/drivers/thunderbolt/icm/icm_nhi.c
index 23047d3..a849698 100644
--- a/drivers/thunderbolt/icm/icm_nhi.c
+++ b/drivers/thunderbolt/icm/icm_nhi.c
@@ -64,6 +64,13 @@ static const struct nla_policy nhi_genl_policy[NHI_ATTR_MAX + 1] = {
.len = TBT_ICM_RING_MAX_FRAME_SIZE },
[NHI_ATTR_MSG_FROM_ICM] = { .type = NLA_BINARY,
.len = TBT_ICM_RING_MAX_FRAME_SIZE },
+ [NHI_ATTR_LOCAL_ROUTE_STRING] = {
+ .len = sizeof(struct route_string) },
+ [NHI_ATTR_LOCAL_UUID] = { .len = sizeof(uuid_be) },
+ [NHI_ATTR_REMOTE_UUID] = { .len = sizeof(uuid_be) },
+ [NHI_ATTR_LOCAL_DEPTH] = { .type = NLA_U8, },
+ [NHI_ATTR_ENABLE_FULL_E2E] = { .type = NLA_FLAG, },
+ [NHI_ATTR_MATCH_FRAME_ID] = { .type = NLA_FLAG, },
};
/* NHI genetlink family */
@@ -480,6 +487,29 @@ int nhi_mailbox(struct tbt_nhi_ctxt *nhi_ctxt, u32 cmd, u32 data, bool deinit)
return 0;
}
+static inline bool nhi_is_path_disconnected(u32 cmd, u8 num_ports)
+{
+ return (cmd >= DISCONNECT_PORT_A_INTER_DOMAIN_PATH &&
+ cmd < (DISCONNECT_PORT_A_INTER_DOMAIN_PATH + num_ports));
+}
+
+static int nhi_mailbox_disconn_path(struct tbt_nhi_ctxt *nhi_ctxt, u32 cmd)
+ __releases(&controllers_list_mutex)
+{
+ struct port_net_dev *port;
+ u32 port_num = cmd - DISCONNECT_PORT_A_INTER_DOMAIN_PATH;
+
+ port = &(nhi_ctxt->net_devices[port_num]);
+ mutex_lock(&port->state_mutex);
+
+ mutex_unlock(&controllers_list_mutex);
+ port->medium_sts = MEDIUM_READY_FOR_APPROVAL;
+ if (port->net_dev)
+ negotiation_events(port->net_dev, MEDIUM_DISCONNECTED);
+ mutex_unlock(&port->state_mutex);
+ return 0;
+}
+
static int nhi_mailbox_generic(struct tbt_nhi_ctxt *nhi_ctxt, u32 mb_cmd)
__releases(&controllers_list_mutex)
{
@@ -526,13 +556,90 @@ static int nhi_genl_mailbox(__always_unused struct sk_buff *u_skb,
return -ERESTART;
nhi_ctxt = nhi_search_ctxt(*(u32 *)info->userhdr);
- if (nhi_ctxt && !nhi_ctxt->d0_exit)
- return nhi_mailbox_generic(nhi_ctxt, mb_cmd);
+ if (nhi_ctxt && !nhi_ctxt->d0_exit) {
+
+ /* rwsem is released later by the below functions */
+ if (nhi_is_path_disconnected(cmd, nhi_ctxt->num_ports))
+ return nhi_mailbox_disconn_path(nhi_ctxt, cmd);
+ else
+ return nhi_mailbox_generic(nhi_ctxt, mb_cmd);
+
+ }
mutex_unlock(&controllers_list_mutex);
return -ENODEV;
}
+static int nhi_genl_approve_networking(__always_unused struct sk_buff *u_skb,
+ struct genl_info *info)
+{
+ struct tbt_nhi_ctxt *nhi_ctxt;
+ struct route_string *route_str;
+ int res = -ENODEV;
+ u8 port_num;
+
+ if (!info || !info->userhdr || !info->attrs ||
+ !info->attrs[NHI_ATTR_LOCAL_ROUTE_STRING] ||
+ !info->attrs[NHI_ATTR_LOCAL_UUID] ||
+ !info->attrs[NHI_ATTR_REMOTE_UUID] ||
+ !info->attrs[NHI_ATTR_LOCAL_DEPTH])
+ return -EINVAL;
+
+ /*
+ * route_str is an unique topological address
+ * used for approving remote controller
+ */
+ route_str = nla_data(info->attrs[NHI_ATTR_LOCAL_ROUTE_STRING]);
+ /* extracts the port we're connected to */
+ port_num = PORT_NUM_FROM_LINK(L0_PORT_NUM(route_str->lo));
+
+ if (mutex_lock_interruptible(&controllers_list_mutex))
+ return -ERESTART;
+
+ nhi_ctxt = nhi_search_ctxt(*(u32 *)info->userhdr);
+ if (nhi_ctxt && !nhi_ctxt->d0_exit) {
+ struct port_net_dev *port;
+
+ if (port_num >= nhi_ctxt->num_ports) {
+ res = -EINVAL;
+ goto free_ctl_list;
+ }
+
+ port = &(nhi_ctxt->net_devices[port_num]);
+
+ mutex_lock(&port->state_mutex);
+ mutex_unlock(&controllers_list_mutex);
+
+ if (port->medium_sts != MEDIUM_READY_FOR_APPROVAL)
+ goto unlock;
+
+ port->medium_sts = MEDIUM_READY_FOR_CONNECTION;
+
+ if (!port->net_dev) {
+ port->net_dev = nhi_alloc_etherdev(nhi_ctxt, port_num,
+ info);
+ if (!port->net_dev) {
+ mutex_unlock(&port->state_mutex);
+ return -ENOMEM;
+ }
+ } else {
+ nhi_update_etherdev(nhi_ctxt, port->net_dev, info);
+
+ negotiation_events(port->net_dev,
+ MEDIUM_READY_FOR_CONNECTION);
+ }
+
+unlock:
+ mutex_unlock(&port->state_mutex);
+
+ return 0;
+ }
+
+free_ctl_list:
+ mutex_unlock(&controllers_list_mutex);
+
+ return res;
+}
static int nhi_genl_send_msg(struct tbt_nhi_ctxt *nhi_ctxt, enum pdf_value pdf,
const u8 *msg, u32 msg_len)
@@ -579,17 +686,127 @@ genl_put_reply_failure:
return res;
}
+static bool nhi_handle_inter_domain_msg(struct tbt_nhi_ctxt *nhi_ctxt,
+ struct thunderbolt_ip_header *hdr)
+{
+ struct port_net_dev *port;
+ u8 port_num;
+
+ const uuid_be proto_uuid = APPLE_THUNDERBOLT_IP_PROTOCOL_UUID;
+
+ if (uuid_be_cmp(proto_uuid, hdr->apple_tbt_ip_proto_uuid) != 0)
+ return true;
+
+ port_num = PORT_NUM_FROM_LINK(
+ L0_PORT_NUM(be32_to_cpu(hdr->route_str.lo)));
+
+ if (unlikely(port_num >= nhi_ctxt->num_ports))
+ return false;
+
+ port = &(nhi_ctxt->net_devices[port_num]);
+ mutex_lock(&port->state_mutex);
+ if (port->net_dev != NULL)
+ negotiation_messages(port->net_dev, hdr);
+ mutex_unlock(&port->state_mutex);
+
+ return false;
+}
+
+static void nhi_handle_notification_msg(struct tbt_nhi_ctxt *nhi_ctxt,
+ const u8 *msg)
+{
+ struct port_net_dev *port;
+ u8 port_num;
+
+#define INTER_DOMAIN_LINK_SHIFT 0
+#define INTER_DOMAIN_LINK_MASK GENMASK(2, INTER_DOMAIN_LINK_SHIFT)
+ switch (msg[3]) {
+
+ case NC_INTER_DOMAIN_CONNECTED:
+ port_num = PORT_NUM_FROM_MSG(msg[5]);
+#define INTER_DOMAIN_APPROVED BIT(3)
+ if (port_num < nhi_ctxt->num_ports &&
+ !(msg[5] & INTER_DOMAIN_APPROVED))
+ nhi_ctxt->net_devices[port_num].medium_sts =
+ MEDIUM_READY_FOR_APPROVAL;
+ break;
+
+ case NC_INTER_DOMAIN_DISCONNECTED:
+ port_num = PORT_NUM_FROM_MSG(msg[5]);
+
+ if (unlikely(port_num >= nhi_ctxt->num_ports))
+ break;
+
+ port = &(nhi_ctxt->net_devices[port_num]);
+ mutex_lock(&port->state_mutex);
+ port->medium_sts = MEDIUM_DISCONNECTED;
+
+ if (port->net_dev != NULL)
+ negotiation_events(port->net_dev,
+ MEDIUM_DISCONNECTED);
+ mutex_unlock(&port->state_mutex);
+ break;
+ }
+}
+
+static bool nhi_handle_icm_response_msg(struct tbt_nhi_ctxt *nhi_ctxt,
+ const u8 *msg)
+{
+ struct port_net_dev *port;
+ bool send_event = true;
+ u8 port_num;
+
+ if (nhi_ctxt->ignore_icm_resp &&
+ msg[3] == RC_INTER_DOMAIN_PKT_SENT) {
+ nhi_ctxt->ignore_icm_resp = false;
+ send_event = false;
+ }
+ if (nhi_ctxt->wait_for_icm_resp) {
+ nhi_ctxt->wait_for_icm_resp = false;
+ up(&nhi_ctxt->send_sem);
+ }
+
+ if (msg[3] == RC_APPROVE_INTER_DOMAIN_CONNECTION) {
+#define APPROVE_INTER_DOMAIN_ERROR BIT(0)
+ if (unlikely(msg[2] & APPROVE_INTER_DOMAIN_ERROR))
+ return send_event;
+
+ port_num = PORT_NUM_FROM_LINK((msg[5]&INTER_DOMAIN_LINK_MASK)>>
+ INTER_DOMAIN_LINK_SHIFT);
+
+ if (unlikely(port_num >= nhi_ctxt->num_ports))
+ return send_event;
+
+ port = &(nhi_ctxt->net_devices[port_num]);
+ mutex_lock(&port->state_mutex);
+ port->medium_sts = MEDIUM_CONNECTED;
+
+ if (port->net_dev != NULL)
+ negotiation_events(port->net_dev, MEDIUM_CONNECTED);
+ mutex_unlock(&port->state_mutex);
+ }
+
+ return send_event;
+}
+
static bool nhi_msg_from_icm_analysis(struct tbt_nhi_ctxt *nhi_ctxt,
enum pdf_value pdf,
const u8 *msg, u32 msg_len)
{
- /*
- * preparation for messages that won't be sent,
- * currently unused in this patch.
- */
bool send_event = true;
switch (pdf) {
+ case PDF_INTER_DOMAIN_REQUEST:
+ case PDF_INTER_DOMAIN_RESPONSE:
+ send_event = nhi_handle_inter_domain_msg(
+ nhi_ctxt,
+ (struct thunderbolt_ip_header *)msg);
+ break;
+
+ case PDF_FW_TO_SW_NOTIFICATION:
+ nhi_handle_notification_msg(nhi_ctxt, msg);
+ break;
+
case PDF_ERROR_NOTIFICATION:
/* fallthrough */
case PDF_WRITE_CONFIGURATION_REGISTERS:
@@ -599,7 +816,12 @@ static bool nhi_msg_from_icm_analysis(struct tbt_nhi_ctxt *nhi_ctxt,
nhi_ctxt->wait_for_icm_resp = false;
up(&nhi_ctxt->send_sem);
}
- /* fallthrough */
+ break;
+
+ case PDF_FW_TO_SW_RESPONSE:
+ send_event = nhi_handle_icm_response_msg(nhi_ctxt, msg);
+ break;
+
default:
break;
}
@@ -788,6 +1010,12 @@ static const struct genl_ops nhi_ops[] = {
.doit = nhi_genl_mailbox,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NHI_CMD_APPROVE_TBT_NETWORKING,
+ .policy = nhi_genl_policy,
+ .doit = nhi_genl_approve_networking,
+ .flags = GENL_ADMIN_PERM,
+ },
};
static int nhi_suspend(struct device *dev) __releases(&nhi_ctxt->send_sem)
@@ -795,6 +1023,17 @@ static int nhi_suspend(struct device *dev) __releases(&nhi_ctxt->send_sem)
struct tbt_nhi_ctxt *nhi_ctxt = pci_get_drvdata(to_pci_dev(dev));
void __iomem *rx_reg, *tx_reg;
u32 rx_reg_val, tx_reg_val;
+ int i;
+
+ for (i = 0; i < nhi_ctxt->num_ports; i++) {
+ struct port_net_dev *port = &nhi_ctxt->net_devices[i];
+
+ mutex_lock(&port->state_mutex);
+ port->medium_sts = MEDIUM_DISCONNECTED;
+ if (port->net_dev)
+ negotiation_events(port->net_dev, MEDIUM_DISCONNECTED);
+ mutex_unlock(&port->state_mutex);
+ }
/* must be after negotiation_events, since messages might be sent */
nhi_ctxt->d0_exit = true;
@@ -954,6 +1193,15 @@ static void icm_nhi_remove(struct pci_dev *pdev)
nhi_suspend(&pdev->dev);
+ for (i = 0; i < nhi_ctxt->num_ports; i++) {
+ mutex_lock(&nhi_ctxt->net_devices[i].state_mutex);
+ if (nhi_ctxt->net_devices[i].net_dev) {
+ nhi_dealloc_etherdev(nhi_ctxt->net_devices[i].net_dev);
+ nhi_ctxt->net_devices[i].net_dev = NULL;
+ }
+ mutex_unlock(&nhi_ctxt->net_devices[i].state_mutex);
+ }
+
if (nhi_ctxt->net_workqueue)
destroy_workqueue(nhi_ctxt->net_workqueue);
diff --git a/drivers/thunderbolt/icm/net.c b/drivers/thunderbolt/icm/net.c
new file mode 100644
index 0000000..beeafb3
--- /dev/null
+++ b/drivers/thunderbolt/icm/net.c
@@ -0,0 +1,783 @@
+/*******************************************************************************
+ *
+ * Intel Thunderbolt(TM) driver
+ * Copyright(c) 2014 - 2016 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ ******************************************************************************/
+
+#include <linux/etherdevice.h>
+#include <linux/crc32.h>
+#include <linux/prefetch.h>
+#include <linux/highmem.h>
+#include <linux/if_vlan.h>
+#include <linux/jhash.h>
+#include <linux/vmalloc.h>
+#include <net/ip6_checksum.h>
+#include "icm_nhi.h"
+#include "net.h"
+
+#define DEFAULT_MSG_ENABLE (NETIF_MSG_PROBE | NETIF_MSG_LINK | NETIF_MSG_IFUP)
+static int debug = -1;
+module_param(debug, int, 0000);
+MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+
+#define TBT_NET_RX_HDR_SIZE 256
+
+#define NUM_TX_LOGIN_RETRIES 60
+
+#define APPLE_THUNDERBOLT_IP_PROTOCOL_REVISION 1
+
+#define LOGIN_TX_PATH 0xf
+
+#define TBT_NET_MTU (64 * 1024)
+
+/* Number of Rx buffers we bundle into one write to the hardware */
+#define TBT_NET_RX_BUFFER_WRITE 16
+
+#define TBT_NET_MULTICAST_HASH_TABLE_SIZE 1024
+#define TBT_NET_ETHER_ADDR_HASH(addr) (((addr[4] >> 4) | (addr[5] << 4)) % \
+ TBT_NET_MULTICAST_HASH_TABLE_SIZE)
+
+#define BITS_PER_U32 (sizeof(u32) * BITS_PER_BYTE)
+
+#define TBT_NET_NUM_TX_BUFS 256
+#define TBT_NET_NUM_RX_BUFS 256
+#define TBT_NET_SIZE_TOTAL_DESCS ((TBT_NET_NUM_TX_BUFS + TBT_NET_NUM_RX_BUFS) \
+ * sizeof(struct tbt_buf_desc))
+
+
+#define TBT_NUM_FRAMES_PER_PAGE (PAGE_SIZE / TBT_RING_MAX_FRAME_SIZE)
+
+#define TBT_NUM_BUFS_BETWEEN(idx1, idx2, num_bufs) \
+ (((num_bufs) - 1) - \
+ ((((idx1) - (idx2)) + (num_bufs)) & ((num_bufs) - 1)))
+
+#define TX_WAKE_THRESHOLD (2 * DIV_ROUND_UP(TBT_NET_MTU, \
+ TBT_RING_MAX_FRM_DATA_SZ))
+
+#define TBT_NET_DESC_ATTR_SOF_EOF (((PDF_TBT_NET_START_OF_FRAME << \
+ DESC_ATTR_SOF_SHIFT) & \
+ DESC_ATTR_SOF_MASK) | \
+ ((PDF_TBT_NET_END_OF_FRAME << \
+ DESC_ATTR_EOF_SHIFT) & \
+ DESC_ATTR_EOF_MASK))
+
+/* E2E workaround */
+#define TBT_EXIST_BUT_UNUSED_HOPID 2
+
+enum tbt_net_frame_pdf {
+ PDF_TBT_NET_MIDDLE_FRAME,
+ PDF_TBT_NET_START_OF_FRAME,
+ PDF_TBT_NET_END_OF_FRAME,
+};
+
+struct thunderbolt_ip_login {
+ struct thunderbolt_ip_header header;
+ __be32 protocol_revision;
+ __be32 transmit_path;
+ __be32 reserved[4];
+ __be32 crc;
+};
+
+struct thunderbolt_ip_login_response {
+ struct thunderbolt_ip_header header;
+ __be32 status;
+ __be32 receiver_mac_address[2];
+ __be32 receiver_mac_address_length;
+ __be32 reserved[4];
+ __be32 crc;
+};
+
+struct thunderbolt_ip_logout {
+ struct thunderbolt_ip_header header;
+ __be32 crc;
+};
+
+struct thunderbolt_ip_status {
+ struct thunderbolt_ip_header header;
+ __be32 status;
+ __be32 crc;
+};
+
+struct approve_inter_domain_connection_cmd {
+ __be32 req_code;
+ __be32 attributes;
+#define AIDC_ATTR_LINK_SHIFT 16
+#define AIDC_ATTR_LINK_MASK GENMASK(18, AIDC_ATTR_LINK_SHIFT)
+#define AIDC_ATTR_DEPTH_SHIFT 20
+#define AIDC_ATTR_DEPTH_MASK GENMASK(23, AIDC_ATTR_DEPTH_SHIFT)
+ uuid_be remote_uuid;
+ __be16 transmit_ring_number;
+ __be16 transmit_path;
+ __be16 receive_ring_number;
+ __be16 receive_path;
+ __be32 crc;
+
+};
+
+enum neg_event {
+ RECEIVE_LOGOUT = NUM_MEDIUM_STATUSES,
+ RECEIVE_LOGIN_RESPONSE,
+ RECEIVE_LOGIN,
+ NUM_NEG_EVENTS
+};
+
+enum disconnect_path_stage {
+ STAGE_1 = BIT(0),
+ STAGE_2 = BIT(1)
+};
+
+/**
+ * struct tbt_port - the basic tbt_port structure
+ * @tbt_nhi_ctxt: context of the nhi controller.
+ * @net_dev: networking device object.
+ * @login_retry_work: work queue for sending login requests.
+ * @login_response_work: work queue for sending login responses.
+ * @work_struct logout_work: work queue for sending logout requests.
+ * @status_reply_work: work queue for sending logout replies.
+ * @approve_inter_domain_work: work queue for sending interdomain to icm.
+ * @route_str: allows to route the messages to destination.
+ * @interdomain_local_uuid: allows to route the messages from local source.
+ * @interdomain_remote_uuid: allows to route the messages to destination.
+ * @command_id a number that identifies the command.
+ * @negotiation_status: holds the network negotiation state.
+ * @msg_enable: used for debugging filters.
+ * @seq_num: a number that identifies the session.
+ * @login_retry_count: counts number of login retries sent.
+ * @local_depth: depth of the remote peer in the chain.
+ * @transmit_path: routing parameter for the icm.
+ * @frame_id: counting ID of frames.
+ * @num: port number.
+ * @local_path: routing parameter for the icm.
+ * @enable_full_e2e: whether to enable full E2E.
+ * @match_frame_id: whether to match frame id on incoming packets.
+ */
+struct tbt_port {
+ struct tbt_nhi_ctxt *nhi_ctxt;
+ struct net_device *net_dev;
+ struct delayed_work login_retry_work;
+ struct work_struct login_response_work;
+ struct work_struct logout_work;
+ struct work_struct status_reply_work;
+ struct work_struct approve_inter_domain_work;
+ struct route_string route_str;
+ uuid_be interdomain_local_uuid;
+ uuid_be interdomain_remote_uuid;
+ u32 command_id;
+ u16 negotiation_status;
+ u16 msg_enable;
+ u8 seq_num;
+ u8 login_retry_count;
+ u8 local_depth;
+ u8 transmit_path;
+ u16 frame_id;
+ u8 num;
+ u8 local_path;
+ bool enable_full_e2e : 1;
+ bool match_frame_id : 1;
+};
+
+static void disconnect_path(struct tbt_port *port,
+ enum disconnect_path_stage stage)
+{
+ u32 cmd = (DISCONNECT_PORT_A_INTER_DOMAIN_PATH + port->num);
+
+ cmd <<= REG_INMAIL_CMD_CMD_SHIFT;
+ cmd &= REG_INMAIL_CMD_CMD_MASK;
+ cmd |= REG_INMAIL_CMD_REQUEST;
+
+ mutex_lock(&port->nhi_ctxt->mailbox_mutex);
+ if (!mutex_trylock(&port->nhi_ctxt->d0_exit_mailbox_mutex)) {
+ netif_notice(port, link, port->net_dev, "controller id %#x is existing D0\n",
+ port->nhi_ctxt->id);
+ } else {
+ nhi_mailbox(port->nhi_ctxt, cmd, stage, false);
+
+ port->nhi_ctxt->net_devices[port->num].medium_sts =
+ MEDIUM_READY_FOR_CONNECTION;
+
+ mutex_unlock(&port->nhi_ctxt->d0_exit_mailbox_mutex);
+ }
+ mutex_unlock(&port->nhi_ctxt->mailbox_mutex);
+}
+
+static void tbt_net_tear_down(struct net_device *net_dev, bool send_logout)
+{
+ struct tbt_port *port = netdev_priv(net_dev);
+ void __iomem *iobase = port->nhi_ctxt->iobase;
+ void __iomem *tx_reg = NULL;
+ u32 tx_reg_val = 0;
+
+ netif_carrier_off(net_dev);
+ netif_stop_queue(net_dev);
+
+ if (port->negotiation_status & BIT(MEDIUM_CONNECTED)) {
+ void __iomem *rx_reg = iobase + REG_RX_OPTIONS_BASE +
+ (port->local_path * REG_OPTS_STEP);
+ u32 rx_reg_val = ioread32(rx_reg) & ~REG_OPTS_E2E_EN;
+
+ tx_reg = iobase + REG_TX_OPTIONS_BASE +
+ (port->local_path * REG_OPTS_STEP);
+ tx_reg_val = ioread32(tx_reg) & ~REG_OPTS_E2E_EN;
+
+ disconnect_path(port, STAGE_1);
+
+ /* disable RX flow control */
+ iowrite32(rx_reg_val, rx_reg);
+ /* disable TX flow control */
+ iowrite32(tx_reg_val, tx_reg);
+ /* disable RX ring */
+ iowrite32(rx_reg_val & ~REG_OPTS_VALID, rx_reg);
+
+ rx_reg = iobase + REG_RX_RING_BASE +
+ (port->local_path * REG_RING_STEP);
+ iowrite32(0, rx_reg + REG_RING_PHYS_LO_OFFSET);
+ iowrite32(0, rx_reg + REG_RING_PHYS_HI_OFFSET);
+ }
+
+ /* Stop login messages */
+ cancel_delayed_work_sync(&port->login_retry_work);
+
+ if (send_logout)
+ queue_work(port->nhi_ctxt->net_workqueue, &port->logout_work);
+
+ if (port->negotiation_status & BIT(MEDIUM_CONNECTED)) {
+ unsigned long flags;
+
+ /* wait for TX to finish */
+ usleep_range(5 * USEC_PER_MSEC, 7 * USEC_PER_MSEC);
+ /* disable TX ring */
+ iowrite32(tx_reg_val & ~REG_OPTS_VALID, tx_reg);
+
+ disconnect_path(port, STAGE_2);
+
+ spin_lock_irqsave(&port->nhi_ctxt->lock, flags);
+ /* disable RX and TX interrupts */
+ RING_INT_DISABLE_TX_RX(iobase, port->local_path,
+ port->nhi_ctxt->num_paths);
+ spin_unlock_irqrestore(&port->nhi_ctxt->lock, flags);
+ }
+}
+
+static inline int send_message(struct tbt_port *port, const char *func,
+ enum pdf_value pdf, u32 msg_len,
+ const void *msg)
+{
+ u32 crc_offset = msg_len - sizeof(__be32);
+ __be32 *crc = (__be32 *)((u8 *)msg + crc_offset);
+ bool is_intdom = (pdf == PDF_INTER_DOMAIN_RESPONSE);
+ int res;
+
+ *crc = cpu_to_be32(~__crc32c_le(~0, msg, crc_offset));
+ res = down_timeout(&port->nhi_ctxt->send_sem,
+ msecs_to_jiffies(3 * MSEC_PER_SEC));
+ if (res) {
+ netif_err(port, link, port->net_dev, "%s: controller id %#x timeout on send semaphore\n",
+ func, port->nhi_ctxt->id);
+ return res;
+ }
+
+ if (!mutex_trylock(&port->nhi_ctxt->d0_exit_send_mutex)) {
+ up(&port->nhi_ctxt->send_sem);
+ netif_notice(port, link, port->net_dev, "%s: controller id %#x is existing D0\n",
+ func, port->nhi_ctxt->id);
+ return -ENODEV;
+ }
+
+ res = nhi_send_message(port->nhi_ctxt, pdf, msg_len, msg, is_intdom);
+
+ mutex_unlock(&port->nhi_ctxt->d0_exit_send_mutex);
+ if (res)
+ up(&port->nhi_ctxt->send_sem);
+
+ return res;
+}
+
+static void approve_inter_domain(struct work_struct *work)
+{
+ struct tbt_port *port = container_of(work, typeof(*port),
+ approve_inter_domain_work);
+ struct approve_inter_domain_connection_cmd approve_msg = {
+ .req_code = cpu_to_be32(CC_APPROVE_INTER_DOMAIN_CONNECTION),
+ .transmit_path = cpu_to_be16(LOGIN_TX_PATH),
+ };
+ u32 aidc = (L0_PORT_NUM(port->route_str.lo) << AIDC_ATTR_LINK_SHIFT) &
+ AIDC_ATTR_LINK_MASK;
+
+ aidc |= (port->local_depth << AIDC_ATTR_DEPTH_SHIFT) &
+ AIDC_ATTR_DEPTH_MASK;
+
+ approve_msg.attributes = cpu_to_be32(aidc);
+
+ memcpy(&approve_msg.remote_uuid, &port->interdomain_remote_uuid,
+ sizeof(approve_msg.remote_uuid));
+ approve_msg.transmit_ring_number = cpu_to_be16(port->local_path);
+ approve_msg.receive_ring_number = cpu_to_be16(port->local_path);
+ approve_msg.receive_path = cpu_to_be16(port->transmit_path);
+
+ send_message(port, __func__, PDF_SW_TO_FW_COMMAND, sizeof(approve_msg),
+ &approve_msg);
+}
+
+static inline void prepare_header(struct thunderbolt_ip_header *header,
+ struct tbt_port *port,
+ enum thunderbolt_ip_packet_type packet_type,
+ u8 len_dwords)
+{
+ const uuid_be proto_uuid = APPLE_THUNDERBOLT_IP_PROTOCOL_UUID;
+
+ header->packet_type = cpu_to_be32(packet_type);
+ header->route_str.hi = cpu_to_be32(port->route_str.hi);
+ header->route_str.lo = cpu_to_be32(port->route_str.lo);
+ header->attributes = cpu_to_be32(
+ ((port->seq_num << HDR_ATTR_SEQ_NUM_SHIFT) &
+ HDR_ATTR_SEQ_NUM_MASK) |
+ ((len_dwords << HDR_ATTR_LEN_SHIFT) & HDR_ATTR_LEN_MASK));
+ memcpy(&header->apple_tbt_ip_proto_uuid, &proto_uuid,
+ sizeof(header->apple_tbt_ip_proto_uuid));
+ memcpy(&header->initiator_uuid, &port->interdomain_local_uuid,
+ sizeof(header->initiator_uuid));
+ memcpy(&header->target_uuid, &port->interdomain_remote_uuid,
+ sizeof(header->target_uuid));
+ header->command_id = cpu_to_be32(port->command_id);
+
+ port->command_id++;
+}
+
+static void status_reply(struct work_struct *work)
+{
+ struct tbt_port *port = container_of(work, typeof(*port),
+ status_reply_work);
+ struct thunderbolt_ip_status status_msg = {
+ .status = 0,
+ };
+
+ prepare_header(&status_msg.header, port,
+ THUNDERBOLT_IP_STATUS_TYPE,
+ (offsetof(struct thunderbolt_ip_status, crc) -
+ offsetof(struct thunderbolt_ip_status,
+ header.apple_tbt_ip_proto_uuid)) /
+ sizeof(u32));
+
+ send_message(port, __func__, PDF_INTER_DOMAIN_RESPONSE,
+ sizeof(status_msg), &status_msg);
+
+}
+
+static void logout(struct work_struct *work)
+{
+ struct tbt_port *port = container_of(work, typeof(*port),
+ logout_work);
+ struct thunderbolt_ip_logout logout_msg;
+
+ prepare_header(&logout_msg.header, port,
+ THUNDERBOLT_IP_LOGOUT_TYPE,
+ (offsetof(struct thunderbolt_ip_logout, crc) -
+ offsetof(struct thunderbolt_ip_logout,
+ header.apple_tbt_ip_proto_uuid)) / sizeof(u32));
+
+ send_message(port, __func__, PDF_INTER_DOMAIN_RESPONSE,
+ sizeof(logout_msg), &logout_msg);
+
+}
+
+static void login_response(struct work_struct *work)
+{
+ struct tbt_port *port = container_of(work, typeof(*port),
+ login_response_work);
+ struct thunderbolt_ip_login_response login_res_msg = {
+ .receiver_mac_address_length = cpu_to_be32(ETH_ALEN),
+ };
+
+ prepare_header(&login_res_msg.header, port,
+ THUNDERBOLT_IP_LOGIN_RESPONSE_TYPE,
+ (offsetof(struct thunderbolt_ip_login_response, crc) -
+ offsetof(struct thunderbolt_ip_login_response,
+ header.apple_tbt_ip_proto_uuid)) / sizeof(u32));
+
+ ether_addr_copy((u8 *)login_res_msg.receiver_mac_address,
+ port->net_dev->dev_addr);
+
+ send_message(port, __func__, PDF_INTER_DOMAIN_RESPONSE,
+ sizeof(login_res_msg), &login_res_msg);
+
+}
+
+static void login_retry(struct work_struct *work)
+{
+ struct tbt_port *port = container_of(work, typeof(*port),
+ login_retry_work.work);
+ struct thunderbolt_ip_login login_msg = {
+ .protocol_revision = cpu_to_be32(
+ APPLE_THUNDERBOLT_IP_PROTOCOL_REVISION),
+ .transmit_path = cpu_to_be32(LOGIN_TX_PATH),
+ };
+
+
+ if (port->nhi_ctxt->d0_exit)
+ return;
+
+ port->login_retry_count++;
+
+ prepare_header(&login_msg.header, port,
+ THUNDERBOLT_IP_LOGIN_TYPE,
+ (offsetof(struct thunderbolt_ip_login, crc) -
+ offsetof(struct thunderbolt_ip_login,
+ header.apple_tbt_ip_proto_uuid)) / sizeof(u32));
+
+ if (send_message(port, __func__, PDF_INTER_DOMAIN_RESPONSE,
+ sizeof(login_msg), &login_msg) == -ENODEV)
+ return;
+
+ if (likely(port->login_retry_count < NUM_TX_LOGIN_RETRIES))
+ queue_delayed_work(port->nhi_ctxt->net_workqueue,
+ &port->login_retry_work,
+ msecs_to_jiffies(5 * MSEC_PER_SEC));
+ else
+ netif_notice(port, link, port->net_dev, "port %u (%#x) login timeout after %u retries\n",
+ port->num, port->negotiation_status,
+ port->login_retry_count);
+}
+
+void negotiation_events(struct net_device *net_dev,
+ enum medium_status medium_sts)
+{
+ struct tbt_port *port = netdev_priv(net_dev);
+ void __iomem *iobase = port->nhi_ctxt->iobase;
+ u32 sof_eof_en, tx_ring_conf, rx_ring_conf, e2e_en;
+ void __iomem *reg;
+ unsigned long flags;
+ u16 hop_id;
+ bool send_logout;
+
+ if (!netif_running(net_dev)) {
+ netif_dbg(port, link, net_dev, "port %u (%#x) is down\n",
+ port->num, port->negotiation_status);
+ return;
+ }
+
+ netif_dbg(port, link, net_dev, "port %u (%#x) receive event %u\n",
+ port->num, port->negotiation_status, medium_sts);
+
+ switch (medium_sts) {
+ case MEDIUM_DISCONNECTED:
+ send_logout = (port->negotiation_status
+ & (BIT(MEDIUM_CONNECTED)
+ | BIT(MEDIUM_READY_FOR_CONNECTION)));
+ send_logout = send_logout && !(port->negotiation_status &
+ BIT(RECEIVE_LOGOUT));
+
+ tbt_net_tear_down(net_dev, send_logout);
+ port->negotiation_status = BIT(MEDIUM_DISCONNECTED);
+ break;
+
+ case MEDIUM_CONNECTED:
+ /*
+ * check if meanwhile other side sent logout
+ * if yes, just don't allow connection to take place
+ * and disconnect path
+ */
+ if (port->negotiation_status & BIT(RECEIVE_LOGOUT)) {
+ disconnect_path(port, STAGE_1 | STAGE_2);
+ break;
+ }
+
+ port->negotiation_status = BIT(MEDIUM_CONNECTED);
+
+ /* configure TX ring */
+ reg = iobase + REG_TX_RING_BASE +
+ (port->local_path * REG_RING_STEP);
+
+ tx_ring_conf = (TBT_NET_NUM_TX_BUFS << REG_RING_SIZE_SHIFT) &
+ REG_RING_SIZE_MASK;
+
+ iowrite32(tx_ring_conf, reg + REG_RING_SIZE_OFFSET);
+
+ /* enable the rings */
+ reg = iobase + REG_TX_OPTIONS_BASE +
+ (port->local_path * REG_OPTS_STEP);
+ if (port->enable_full_e2e) {
+ iowrite32(REG_OPTS_VALID | REG_OPTS_E2E_EN, reg);
+ hop_id = port->local_path;
+ } else {
+ iowrite32(REG_OPTS_VALID, reg);
+ hop_id = TBT_EXIST_BUT_UNUSED_HOPID;
+ }
+
+ reg = iobase + REG_RX_OPTIONS_BASE +
+ (port->local_path * REG_OPTS_STEP);
+
+ sof_eof_en = (BIT(PDF_TBT_NET_START_OF_FRAME) <<
+ REG_RX_OPTS_MASK_SOF_SHIFT) &
+ REG_RX_OPTS_MASK_SOF_MASK;
+
+ sof_eof_en |= (BIT(PDF_TBT_NET_END_OF_FRAME) <<
+ REG_RX_OPTS_MASK_EOF_SHIFT) &
+ REG_RX_OPTS_MASK_EOF_MASK;
+
+ iowrite32(sof_eof_en, reg + REG_RX_OPTS_MASK_OFFSET);
+
+ e2e_en = REG_OPTS_VALID | REG_OPTS_E2E_EN;
+ e2e_en |= (hop_id << REG_RX_OPTS_TX_E2E_HOP_ID_SHIFT) &
+ REG_RX_OPTS_TX_E2E_HOP_ID_MASK;
+
+ iowrite32(e2e_en, reg);
+
+ /*
+ * Configure RX ring
+ * must be after enable ring for E2E to work
+ */
+ reg = iobase + REG_RX_RING_BASE +
+ (port->local_path * REG_RING_STEP);
+
+ rx_ring_conf = (TBT_NET_NUM_RX_BUFS << REG_RING_SIZE_SHIFT) &
+ REG_RING_SIZE_MASK;
+
+ rx_ring_conf |= (TBT_RING_MAX_FRAME_SIZE <<
+ REG_RING_BUF_SIZE_SHIFT) &
+ REG_RING_BUF_SIZE_MASK;
+
+ iowrite32(rx_ring_conf, reg + REG_RING_SIZE_OFFSET);
+
+ spin_lock_irqsave(&port->nhi_ctxt->lock, flags);
+ /* enable RX interrupt */
+ iowrite32(ioread32(iobase + REG_RING_INTERRUPT_BASE) |
+ REG_RING_INT_RX_PROCESSED(port->local_path,
+ port->nhi_ctxt->num_paths),
+ iobase + REG_RING_INTERRUPT_BASE);
+ spin_unlock_irqrestore(&port->nhi_ctxt->lock, flags);
+
+ netif_info(port, link, net_dev, "Thunderbolt(TM) Networking port %u - ready\n",
+ port->num);
+
+ netif_carrier_on(net_dev);
+ netif_start_queue(net_dev);
+ break;
+
+ case MEDIUM_READY_FOR_CONNECTION:
+ /*
+ * If medium is connected, no reason to go back,
+ * keep it 'connected'.
+ * If received login response, don't need to trigger login
+ * retries again.
+ */
+ if (unlikely(port->negotiation_status &
+ (BIT(MEDIUM_CONNECTED) |
+ BIT(RECEIVE_LOGIN_RESPONSE))))
+ break;
+
+ port->negotiation_status = BIT(MEDIUM_READY_FOR_CONNECTION);
+ port->login_retry_count = 0;
+ queue_delayed_work(port->nhi_ctxt->net_workqueue,
+ &port->login_retry_work, 0);
+ break;
+
+ default:
+ break;
+ }
+}
+
+void negotiation_messages(struct net_device *net_dev,
+ struct thunderbolt_ip_header *hdr)
+{
+ struct tbt_port *port = netdev_priv(net_dev);
+ __be32 status;
+
+ if (!netif_running(net_dev)) {
+ netif_dbg(port, link, net_dev, "port %u (%#x) is down\n",
+ port->num, port->negotiation_status);
+ return;
+ }
+
+ switch (hdr->packet_type) {
+ case cpu_to_be32(THUNDERBOLT_IP_LOGIN_TYPE):
+ port->transmit_path = be32_to_cpu(
+ ((struct thunderbolt_ip_login *)hdr)->transmit_path);
+ netif_dbg(port, link, net_dev, "port %u (%#x) receive ThunderboltIP login message with transmit path %u\n",
+ port->num, port->negotiation_status,
+ port->transmit_path);
+
+ if (unlikely(port->negotiation_status &
+ BIT(MEDIUM_DISCONNECTED)))
+ break;
+
+ queue_work(port->nhi_ctxt->net_workqueue,
+ &port->login_response_work);
+
+ if (unlikely(port->negotiation_status & BIT(MEDIUM_CONNECTED)))
+ break;
+
+ /*
+ * In case a login response received from other peer
+ * on my login and acked their login for the first time,
+ * so just approve the inter-domain now
+ */
+ if (port->negotiation_status & BIT(RECEIVE_LOGIN_RESPONSE)) {
+ if (!(port->negotiation_status & BIT(RECEIVE_LOGIN)))
+ queue_work(port->nhi_ctxt->net_workqueue,
+ &port->approve_inter_domain_work);
+ /*
+ * if we reached the number of max retries or previous
+ * logout, schedule another round of login retries
+ */
+ } else if ((port->login_retry_count >= NUM_TX_LOGIN_RETRIES) ||
+ (port->negotiation_status & BIT(RECEIVE_LOGOUT))) {
+ port->negotiation_status &= ~(BIT(RECEIVE_LOGOUT));
+ port->login_retry_count = 0;
+ queue_delayed_work(port->nhi_ctxt->net_workqueue,
+ &port->login_retry_work, 0);
+ }
+
+ port->negotiation_status |= BIT(RECEIVE_LOGIN);
+
+ break;
+
+ case cpu_to_be32(THUNDERBOLT_IP_LOGIN_RESPONSE_TYPE):
+ status = ((struct thunderbolt_ip_login_response *)hdr)->status;
+ if (likely(status == 0)) {
+ netif_dbg(port, link, net_dev, "port %u (%#x) receive ThunderboltIP login response message\n",
+ port->num,
+ port->negotiation_status);
+
+ if (unlikely(port->negotiation_status &
+ (BIT(MEDIUM_DISCONNECTED) |
+ BIT(MEDIUM_CONNECTED) |
+ BIT(RECEIVE_LOGIN_RESPONSE))))
+ break;
+
+ port->negotiation_status |=
+ BIT(RECEIVE_LOGIN_RESPONSE);
+ cancel_delayed_work_sync(&port->login_retry_work);
+ /*
+ * login was received from other peer and now response
+ * on our login so approve the inter-domain
+ */
+ if (port->negotiation_status & BIT(RECEIVE_LOGIN))
+ queue_work(port->nhi_ctxt->net_workqueue,
+ &port->approve_inter_domain_work);
+ else
+ port->negotiation_status &=
+ ~BIT(RECEIVE_LOGOUT);
+ } else {
+ netif_notice(port, link, net_dev, "port %u (%#x) receive ThunderboltIP login response message with status %u\n",
+ port->num,
+ port->negotiation_status,
+ be32_to_cpu(status));
+ }
+ break;
+
+ case cpu_to_be32(THUNDERBOLT_IP_LOGOUT_TYPE):
+ netif_dbg(port, link, net_dev, "port %u (%#x) receive ThunderboltIP logout message\n",
+ port->num, port->negotiation_status);
+
+ queue_work(port->nhi_ctxt->net_workqueue,
+ &port->status_reply_work);
+ port->negotiation_status &= ~(BIT(RECEIVE_LOGIN) |
+ BIT(RECEIVE_LOGIN_RESPONSE));
+ port->negotiation_status |= BIT(RECEIVE_LOGOUT);
+
+ if (!(port->negotiation_status & BIT(MEDIUM_CONNECTED))) {
+ tbt_net_tear_down(net_dev, false);
+ break;
+ }
+
+ tbt_net_tear_down(net_dev, true);
+
+ port->negotiation_status |= BIT(MEDIUM_READY_FOR_CONNECTION);
+ port->negotiation_status &= ~(BIT(MEDIUM_CONNECTED));
+ break;
+
+ case cpu_to_be32(THUNDERBOLT_IP_STATUS_TYPE):
+ netif_dbg(port, link, net_dev, "port %u (%#x) receive ThunderboltIP status message with status %u\n",
+ port->num, port->negotiation_status,
+ be32_to_cpu(
+ ((struct thunderbolt_ip_status *)hdr)->status));
+ break;
+ }
+}
+
+void nhi_dealloc_etherdev(struct net_device *net_dev)
+{
+ unregister_netdev(net_dev);
+ free_netdev(net_dev);
+}
+
+void nhi_update_etherdev(struct tbt_nhi_ctxt *nhi_ctxt,
+ struct net_device *net_dev, struct genl_info *info)
+{
+ struct tbt_port *port = netdev_priv(net_dev);
+
+ nla_memcpy(&(port->route_str),
+ info->attrs[NHI_ATTR_LOCAL_ROUTE_STRING],
+ sizeof(port->route_str));
+ nla_memcpy(&port->interdomain_remote_uuid,
+ info->attrs[NHI_ATTR_REMOTE_UUID],
+ sizeof(port->interdomain_remote_uuid));
+ port->local_depth = nla_get_u8(info->attrs[NHI_ATTR_LOCAL_DEPTH]);
+ port->enable_full_e2e = nhi_ctxt->support_full_e2e ?
+ nla_get_flag(info->attrs[NHI_ATTR_ENABLE_FULL_E2E]) : false;
+ port->match_frame_id =
+ nla_get_flag(info->attrs[NHI_ATTR_MATCH_FRAME_ID]);
+ port->frame_id = 0;
+}
+
+struct net_device *nhi_alloc_etherdev(struct tbt_nhi_ctxt *nhi_ctxt,
+ u8 port_num, struct genl_info *info)
+{
+ struct tbt_port *port;
+ struct net_device *net_dev = alloc_etherdev(sizeof(struct tbt_port));
+ u32 hash;
+
+ if (!net_dev)
+ return NULL;
+
+ SET_NETDEV_DEV(net_dev, &nhi_ctxt->pdev->dev);
+
+ port = netdev_priv(net_dev);
+ port->nhi_ctxt = nhi_ctxt;
+ port->net_dev = net_dev;
+ nla_memcpy(&port->interdomain_local_uuid,
+ info->attrs[NHI_ATTR_LOCAL_UUID],
+ sizeof(port->interdomain_local_uuid));
+ nhi_update_etherdev(nhi_ctxt, net_dev, info);
+ port->num = port_num;
+ port->local_path = PATH_FROM_PORT(nhi_ctxt->num_paths, port_num);
+
+ port->msg_enable = netif_msg_init(debug, DEFAULT_MSG_ENABLE);
+
+ net_dev->addr_assign_type = NET_ADDR_PERM;
+ /* unicast and locally administred MAC */
+ net_dev->dev_addr[0] = (port_num << 4) | 0x02;
+ hash = jhash2((u32 *)&port->interdomain_local_uuid,
+ sizeof(port->interdomain_local_uuid)/sizeof(u32), 0);
+
+ memcpy(net_dev->dev_addr + 1, &hash, sizeof(hash));
+ hash = jhash2((u32 *)&port->interdomain_local_uuid,
+ sizeof(port->interdomain_local_uuid)/sizeof(u32), hash);
+
+ net_dev->dev_addr[5] = hash & 0xff;
+
+ scnprintf(net_dev->name, sizeof(net_dev->name), "tbtnet%%dp%hhu",
+ port_num);
+
+ INIT_DELAYED_WORK(&port->login_retry_work, login_retry);
+ INIT_WORK(&port->login_response_work, login_response);
+ INIT_WORK(&port->logout_work, logout);
+ INIT_WORK(&port->status_reply_work, status_reply);
+ INIT_WORK(&port->approve_inter_domain_work, approve_inter_domain);
+
+ netif_info(port, probe, net_dev,
+ "Thunderbolt(TM) Networking port %u - MAC Address: %pM\n",
+ port_num, net_dev->dev_addr);
+
+ return net_dev;
+}
diff --git a/drivers/thunderbolt/icm/net.h b/drivers/thunderbolt/icm/net.h
index 0281201..1cb6701 100644
--- a/drivers/thunderbolt/icm/net.h
+++ b/drivers/thunderbolt/icm/net.h
@@ -23,6 +23,10 @@
#include <linux/semaphore.h>
#include <net/genetlink.h>
+#define APPLE_THUNDERBOLT_IP_PROTOCOL_UUID \
+ UUID_BE(0x9E588F79, 0x478A, 0x1636, \
+ 0x64, 0x56, 0xC6, 0x97, 0xDD, 0xC8, 0x20, 0xA9)
+
/*
* Each physical port contains 2 channels.
* Devices are exposed to user based on physical ports.
@@ -33,6 +37,9 @@
* host channel/link which starts from 1.
*/
#define PORT_NUM_FROM_LINK(link) (((link) - 1) / CHANNELS_PER_PORT_NUM)
+#define PORT_NUM_FROM_MSG(msg) PORT_NUM_FROM_LINK(((msg) & \
+ INTER_DOMAIN_LINK_MASK) >> \
+ INTER_DOMAIN_LINK_SHIFT)
#define TBT_TX_RING_FULL(prod, cons, size) ((((prod) + 1) % (size)) == (cons))
#define TBT_TX_RING_EMPTY(prod, cons) ((prod) == (cons))
@@ -125,6 +132,17 @@ enum {
CC_SET_FW_MODE_FDA_DA_ALL
};
+struct route_string {
+ u32 hi;
+ u32 lo;
+};
+
+struct route_string_be {
+ __be32 hi;
+ __be32 lo;
+};
+
+#define L0_PORT_NUM(cpu_route_str_lo) ((cpu_route_str_lo) & GENMASK(5, 0))
/* NHI genetlink attributes */
enum {
@@ -138,12 +156,53 @@ enum {
NHI_ATTR_PDF,
NHI_ATTR_MSG_TO_ICM,
NHI_ATTR_MSG_FROM_ICM,
+ NHI_ATTR_LOCAL_ROUTE_STRING,
+ NHI_ATTR_LOCAL_UUID,
+ NHI_ATTR_REMOTE_UUID,
+ NHI_ATTR_LOCAL_DEPTH,
+ NHI_ATTR_ENABLE_FULL_E2E,
+ NHI_ATTR_MATCH_FRAME_ID,
__NHI_ATTR_MAX,
};
#define NHI_ATTR_MAX (__NHI_ATTR_MAX - 1)
+/* ThunderboltIP Packet Types */
+enum thunderbolt_ip_packet_type {
+ THUNDERBOLT_IP_LOGIN_TYPE,
+ THUNDERBOLT_IP_LOGIN_RESPONSE_TYPE,
+ THUNDERBOLT_IP_LOGOUT_TYPE,
+ THUNDERBOLT_IP_STATUS_TYPE
+};
+
+struct thunderbolt_ip_header {
+ struct route_string_be route_str;
+ __be32 attributes;
+#define HDR_ATTR_LEN_SHIFT 0
+#define HDR_ATTR_LEN_MASK GENMASK(5, HDR_ATTR_LEN_SHIFT)
+#define HDR_ATTR_SEQ_NUM_SHIFT 27
+#define HDR_ATTR_SEQ_NUM_MASK GENMASK(28, HDR_ATTR_SEQ_NUM_SHIFT)
+ uuid_be apple_tbt_ip_proto_uuid;
+ uuid_be initiator_uuid;
+ uuid_be target_uuid;
+ __be32 packet_type;
+ __be32 command_id;
+};
+
+enum medium_status {
+ /* Handle cable disconnection or peer down */
+ MEDIUM_DISCONNECTED,
+ /* Connection is fully established */
+ MEDIUM_CONNECTED,
+ /* Awaiting for being approved by user-space module */
+ MEDIUM_READY_FOR_APPROVAL,
+ /* Approved by user-space, awaiting for establishment flow to finish */
+ MEDIUM_READY_FOR_CONNECTION,
+ NUM_MEDIUM_STATUSES
+};
+
struct port_net_dev {
struct net_device *net_dev;
+ enum medium_status medium_sts;
struct mutex state_mutex;
};
@@ -213,5 +272,16 @@ struct tbt_nhi_ctxt {
int nhi_send_message(struct tbt_nhi_ctxt *nhi_ctxt, enum pdf_value pdf,
u32 msg_len, const void *msg, bool ignore_icm_resp);
int nhi_mailbox(struct tbt_nhi_ctxt *nhi_ctxt, u32 cmd, u32 data, bool deinit);
+struct net_device *nhi_alloc_etherdev(struct tbt_nhi_ctxt *nhi_ctxt,
+ u8 port_num, struct genl_info *info);
+void nhi_update_etherdev(struct tbt_nhi_ctxt *nhi_ctxt,
+ struct net_device *net_dev, struct genl_info *info);
+void nhi_dealloc_etherdev(struct net_device *net_dev);
+void negotiation_events(struct net_device *net_dev,
+ enum medium_status medium_sts);
+void negotiation_messages(struct net_device *net_dev,
+ struct thunderbolt_ip_header *hdr);
+void tbt_net_rx_msi(struct net_device *net_dev);
+void tbt_net_tx_msi(struct net_device *net_dev);
#endif
--
2.7.4
^ 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