* [PATCH 00/13] Enhance stmmac driver to support GMAC4.x IP
From: Alexandre TORGUE @ 2016-03-25 9:14 UTC (permalink / raw)
To: netdev, peppe.cavallaro; +Cc: Lars Persson
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 4976 bytes --]
This is a subset of patch to enhance current stmmac driver to support
new GMAC4.x chips. New set of callbacks is defined to support this new
family: descriptors, dma, core.
One of main changes of GMAC 4.xx IP is descriptors management.
-descriptors are only used in ring mode.
-A descriptor is composed of 4 32bits registers (no more extended
descriptors)
-descriptor mechanism (Tx for example, but it is exactly the same for RX):
-useful registers:
-DMA_CH#_TxDesc_Ring_Len: length of transmit descriptor ring
-DMA_CH#_TxDesc_List_Address: start address of the ring
-DMA_CH#_TxDesc_Tail_Pointer: address of the last descriptor to send + 1.
-DMA_CH#_TxDesc_Current_App_TxDesc: address of the current descriptor
-The descriptor Tail Pointer register contains the pointer to the
descriptor address (N). The base address and the current
descriptor decide the address of the current descriptor that the
DMA can process. The descriptors up to one location less than the
one indicated by the descriptor tail pointer (N-1) are owned by
the DMA. The DMA continues to process the descriptors until the
following condition occurs:
"current descriptor pointer == Descriptor Tail pointer"
Then the DMA goes into suspend mode. The application must perform
a write to descriptor tail pointer register and update the tail
pointer to have the following condition and to start a new transfer:
"current descriptor pointer < Descriptor tail pointer"
The DMA automatically wraps around the base address when the end
of ring is reached.
New features are available on IP:
-TSO (TCP Segmentation Offload) for TX only
-Split header: to have header and payload in 2 different buffers (not yet implemented)
Below some throughput figures obtained on some boxes:
iperf (mbps)
--------------------------------------
tcp udp
tx rx tx rx
-----------------
GMAC4.x 935 930 750 800
Note: There is a change in 4.10a databook on bitfield mapping of DMA_CHANx_INTR_ENA register.
This requires to have é diffrent set of callbacks between IP 4.00a and 4.10a.
Best regards
Alex
Alexandre TORGUE (13):
stmmac: rework get_hw_feature function
stmmac: rework the routines to show the ring status
stmmac: rework synopsys id read, moved to dwmac setup
stmmac: add descriptors function for GMAC 4.xx
stmmac: add GMAC4 DMA/CORE Header File
stmmac: add DMA support for GMAC 4.xx
stmmac: add GMAC4 core support
stmmac: enhance mmc counter management
stmmac: add new DT platform entries for GMAC4
stmmac: support new GMAC4
Documentation: networking: update stmmac
stmmac: update version to Jan_2016
stmmac: update MAINTAINERS
Documentation/devicetree/bindings/net/stmmac.txt | 2 +
Documentation/networking/stmmac.txt | 44 +-
MAINTAINERS | 1 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 3 +-
drivers/net/ethernet/stmicro/stmmac/common.h | 64 +-
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 7 +-
.../net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 35 +-
.../net/ethernet/stmicro/stmmac/dwmac100_core.c | 5 +-
drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 255 ++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 407 +++++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 396 +++++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h | 129 +++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 354 ++++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h | 202 +++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 225 +++++++
drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 21 +
drivers/net/ethernet/stmicro/stmmac/mmc.h | 4 +
drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 349 +++++------
drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 21 +
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 7 +-
.../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 7 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 643 +++++++++++++++------
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +
include/linux/stmmac.h | 2 +
24 files changed, 2821 insertions(+), 369 deletions(-)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac4.h
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
--
1.9.1
^ permalink raw reply
* Re: [PATCH net] net: ipv4: Multipath needs to handle unreachable nexthops
From: Julian Anastasov @ 2016-03-25 9:05 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <56F49CD3.1060406@cumulusnetworks.com>
Hello,
On Thu, 24 Mar 2016, David Ahern wrote:
> On 3/24/16 4:33 PM, Julian Anastasov wrote:
> > But for multipath routes we can also consider the
> > nexthops as "alternatives", so it depends on how one uses
> > the multipath mechanism. The ability to fallback to
> > another nexthop assumes one connection is allowed to
> > move from one ISP to another. What if the second ISP
> > decides to reject the connection? What we have is a
> > broken connection just because the retransmits
> > were diverted to wrong place in the hurry. So, the
> > nexthops can be compatible or incompatible. For your
> > setup they are, for others they are not.
>
> I am not sure I completely understand your point. Are you saying that within a
> single multipath route some connections to nexthops are allowed and others are
> not?
>
> So to put that paragraph into an example
>
> 15.0.0.0/16
> nexthop via 12.0.0.2 dev swp1 weight 1
> nexthop via 12.0.0.3 dev swp1 weight 1
>
> Hosts from 15.0/16 could have TCP connections use 12.0.0.2, but not 12.0.0.3
> because 12.0.0.3 could be a different ISP and not allow TCP connections from
> this address space?
Yes. Two cases are possible:
1. ISP2 filters saddr, traffic with saddr from ISP1 is dropped.
2. ISP2 allows any saddr. But how the responses from
world with daddr=IP_from_ISP1 will come from ISP2 link?
If the nexthops are for different ISP the connection
can survive only if sticks to its ISP. An ISP will
not work as a backup link for another ISP.
The hash-based algorithm does not move connections
from one nexthop to another. If you rearrange the nexthops
on failure, the binding is lost and connections can break.
A fallback from fragile wifi link can upset users and any
redirects will lead to bad experience with broken conns.
So only CONNMARK can help to stick connection to some path.
If this path has multiple simultaneous alternatives you can
again use multipath route reached from 'ip rule from PUBIP2 table ISP2'
but then we again are restricted from the hash-based alg
which is suitable only for default routes hit by saddr=0.0.0.0
lookups. In the common case when connection is created
there are two lookups:
1. TCP selects nexthop with a saddr=0.0.0.0 lookup.
ISP is selected based on the mp alg.
2. If one is lucky to reach ip_route_me_harder the
hash-based lookup is defeated because now lookup
uses saddr=iph->saddr, so it selects different nexthop.
It works while all packets for the connection reach
ip_route_me_harder.
> > So, if the kernel used a random selection
> > your fallback algorithm should help. But it is fragile
> > for the simple setup with single default multipath route.
> > May be what we miss is the ability to choose between
> > random and hash-based selection. Then your patch may be
> > useful but only for setup 2 (multipath route hit only by
> > first packet). So, your patch may come with a sysctl var
> > that explains your current patch logic: "avoid failed nexthops,
> > never probe them, wait their failed entry to be expired by
> > neigh_periodic_work and just then we can use the nexthop
> > by creating new entry to probe the GW". Who will trigger
> > probes often enough to maintain fresh state?
>
> First packet out does the probe -- neigh lookup fails, kernel has no knowledge
> so can't reject the nexthop based on neighbor information.
>
> After that if it has information that says that a nexthop is dead, why would
> it continue to try to probe? Any traffic that selects that nh is dead. That to
If entry becomes FAILED this state is preserved
if we do not direct traffic to this entry. If there was a
single connection that was rejected after 3 failed probes
the next connection (with your patch) will fallback to
another neigh and the first entry will remain in FAILED
state until expiration. If one wants to refresh the state
often, a script/tool that pings all GWs is needed, so that
you can notice the available or failed paths faster.
> me defies the basis of having multiple paths.
We do not know how long is the outage. Long living
connections may prefer to survive with retransmits.
Say you are using SSH via wifi link doing important work.
Do you want your connection to break just because link was
down for a while?
Fallback to other ISP can be unwanted. If we do not
know if multipath route is used per-packet ot per-connection
we can not just apply a fallback to other nexthops.
We can do it only if user can configure the different
usages per multipath route:
1. hash-based or random
2. allow fallback or not. This config is not a MUST if users
can select random mode, it can imply that fallback is allowed.
Regards
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Willy Tarreau @ 2016-03-25 8:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, Yann Ylavic, Linux Kernel Network Developers,
Tolga Ceylan, Craig Gallek, Josh Snyder, Aaron Conole,
David S. Miller, Daniel Borkmann
In-Reply-To: <1458888581.6473.30.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
On Thu, Mar 24, 2016 at 11:49:41PM -0700, Eric Dumazet wrote:
> Everything is possible, but do not complain because BPF went in the
> kernel before your changes.
Don't get me wrong, I'm not complaining, I'm more asking for help to
try to elaborate the alternate solution. I understood well what my
proposal did because it was pretty straightforward, and the new one
I'll have to do is of an immense complexity for me by now, since it
will require learning a new language and finding doc on how all this
works together. But as I said I'm totally sold to the benefits it can
provide for large scale deployments and I'm well aware of the ugly
socket scan there was in the previous one.
> Just rework your patch.
>
> Supporting multiple SO_REUSEPORT groups on the same port should not be
> too hard really. Making sure BPF still work for them is feasible.
>
> But the semantic of the socket option would be really different.
I don't care much about the socket options themselves as long as I can
continue to support seamless reloads. I could even get rid of SO_REUSEPORT
on Linux to use something else instead if I have a reliable way to detect
that the alternative will work.
> You need to not control an individual listener, but a group of listener.
>
> Your dying haproxy would issue a single system call to tell the kernel :
> My SO_REUSEPORT group should not accept new SYN packets, so that the new
> haproxy can setup a working new SO_REUSEPORT group.
Normally it's the other way around :-) The new process first grabs the
socket, there's a tiny window where both are attached, and only then the
old process leaves. That's the only way to ensure there's no loss nor
added latency in the processing.
If you could share a pointer to some good starter documentation for this,
I would appreciate it. I really have no idea where to start from and the
only elements I found on the net didn't give a single hint regarding all
this :-/
Thanks,
Willy
^ permalink raw reply
* Re: [PATCH] stmmac: Fix phy without MDIO subnode
From: Giuseppe CAVALLARO @ 2016-03-25 8:42 UTC (permalink / raw)
To: John Keeping; +Cc: Gabriel Fernandez, netdev, linux-kernel
In-Reply-To: <20160324170119.28d54ed5.john@metanate.com>
On 3/24/2016 6:01 PM, John Keeping wrote:
> On Thu, 24 Mar 2016 13:56:02 +0100, Giuseppe CAVALLARO wrote:
>
>> This should be fixed by some work done some days
>> ago and not yet committed.
>>
>> Pls see "stmmac: MDIO fixes" patch-set and let me know
>> if ok on your side.
>
> Yes, that works for me.
thx John for having tested it on your side.
peppe
>
>
> Thanks,
> John
>
>> On 3/24/2016 11:56 AM, John Keeping wrote:
>>> Since commit 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found'
>>> regression") we no longer allocate mdio_bus_data unless there is a MDIO
>>> subnode. This breaks the ethernet on the Radxa Rock2 (using
>>> rk3288-rock2-square.dts) which does not have an MDIO subnode.
>>>
>>> That commit was correct that the phy_bus_name test is unhelpful since we
>>> allocate "plat" in the same function and never set phy_bus_name so let's
>>> just drop the test which restores the previous behaviour.
>>>
>>> Fixes: 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found' regression")
>>> Signed-off-by: John Keeping <john@metanate.com>
>>> ---
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> index dcbd2a1..e0fa060 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> @@ -189,7 +189,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>>> if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) == 0)
>>> dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>>>
>>> - if ((plat->phy_node && !of_phy_is_fixed_link(np)) || !plat->mdio_node)
>>> + if ((plat->phy_node && !of_phy_is_fixed_link(np)))
>>> plat->mdio_bus_data = NULL;
>>> else
>>> plat->mdio_bus_data =
>>>
>>
>
>
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-25 6:49 UTC (permalink / raw)
To: Willy Tarreau
Cc: Tom Herbert, Yann Ylavic, Linux Kernel Network Developers,
Tolga Ceylan, Craig Gallek, Josh Snyder, Aaron Conole,
David S. Miller, Daniel Borkmann
In-Reply-To: <20160325052819.GA8548@1wt.eu>
On Fri, 2016-03-25 at 06:28 +0100, Willy Tarreau wrote:
> On Thu, Mar 24, 2016 at 04:54:03PM -0700, Tom Herbert wrote:
> > On Thu, Mar 24, 2016 at 4:40 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
> > > I'll learn how to do this to get the best performances from the
> > > server, but having to do so to work around what looks like a defect
> > > (for simple/default SMP configurations at least, no NUMA or clever
> > > CPU-affinity or queuing policy involved) seems odd in the first place.
> > >
> > I disagree with your assessment that there is a defect. SO_REUSEPORT
> > is designed to spread packets amongst _equivalent_ connections. In the
> > server draining case sockets are no longer equivalent, but that is a
> > special case.
>
> I partially disagree with you here Tom. Initially SO_REUSEPORT was not
> used to spread packets but to allow soft restart in some applications.
> I've been using it since 2001 in haproxy on *BSD and linux 2.2. It was
> removed during 2.3 and I used to keep a patch to reimplement it in 2.4
> (basically 2 or 3 lines, the infrastructure was still present), but the
> patch was not accepted. The same patch worked for 2.6 and 3.x, allowing
> me to continue to perform soft-restarts on Linux just like I used to do
> on *BSD. When SO_REUSEPORT was reimplemented in 3.9 with load balancing,
> I was happy because it at last allowed me to drop my patch and I got
> the extra benefit of better load balancing of incoming connections.
>
> But the main use we have for it (at least historically) is for soft
> restarts, where one process replaces another one. Very few people use
> more than one process in our case.
>
> However given the benefits of the load spreading for extreme loads,
> I'm willing to find how to achieve the same with BPF, but it's pretty
> clear that at this point I have no idea where to start from and that
> for a single process replacing a single one, it looks quite complicated.
>
> For me quite frankly the special case is the load balancing which is
> a side benefit (and a nice one, don't get me wrong).
>
> That's why I would have found it nice to "fix" the process replacement
> to avoid dropping incoming connections, though I don't want it to become
> a problem for future improvements on BPF. I don't think the two lines I
> proposed could become an issue but I'll live without them (or continue
> to apply this patch).
>
> BTW, I have no problem with having to write a little bit of assembly for
> fast interfaces if it remains untouched for years, we do already have a
> bit in haproxy. It's just a longterm investment.
Everything is possible, but do not complain because BPF went in the
kernel before your changes.
Just rework your patch.
Supporting multiple SO_REUSEPORT groups on the same port should not be
too hard really. Making sure BPF still work for them is feasible.
But the semantic of the socket option would be really different.
You need to not control an individual listener, but a group of listener.
Your dying haproxy would issue a single system call to tell the kernel :
My SO_REUSEPORT group should not accept new SYN packets, so that the new
haproxy can setup a working new SO_REUSEPORT group.
^ permalink raw reply
* RE: [v6, 2/5] soc: fsl: add GUTS driver for QorIQ platforms
From: Yangbo Lu @ 2016-03-25 6:44 UTC (permalink / raw)
To: Scott Wood, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Zhao Qiang,
Russell King, Bhupesh Sharma, Santosh Shilimkar, Jochen Friedrich,
Yang-Leo Li, Rob Herring, Claudiu Manoil, Kumar Gala, Xiaobo Xie
In-Reply-To: <DB5PR0401MB1928302BB0DDE8D9B47E4361918C0-GXldUsIPo7Z/SeJcUcAJq43W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
> -----Original Message-----
> From: Scott Wood
> Sent: Saturday, March 19, 2016 6:55 AM
> To: Yangbo Lu; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-
> kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-
> dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Rob Herring; Russell King; Jochen Friedrich; Joerg Roedel; Claudiu
> Manoil; ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Bhupesh Sharma; Zhao Qiang; Kumar Gala;
> Santosh Shilimkar; Yang-Leo Li; Xiaobo Xie
> Subject: Re: [v6, 2/5] soc: fsl: add GUTS driver for QorIQ platforms
>
> On 03/09/2016 04:18 AM, Yangbo Lu wrote:
> > +#ifdef CONFIG_FSL_GUTS
> > +u32 fsl_guts_get_svr(void);
> > +int fsl_guts_init(void);
> > +#endif
>
> Don't ifdef prototypes (when not providing a stub alternative).
>
> -Scott
[Lu Yangbo-B47093] Ok, will remove the ifdef.
Thank you very much.
^ permalink raw reply
* RE: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
From: Yangbo Lu @ 2016-03-25 6:43 UTC (permalink / raw)
To: Scott Wood, Arnd Bergmann, Rob Herring
Cc: linuxppc-dev@lists.ozlabs.org, devicetree@vger.kernel.org,
ulf.hansson@linaro.org, Zhao Qiang, Russell King, Bhupesh Sharma,
netdev@vger.kernel.org, Joerg Roedel, Kumar Gala,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
Yang-Leo Li, iommu@lists.linux-foundation.org,
linux-i2c@vger.kernel.org, Claudiu Manoil, Santosh Shilimkar,
Xiaobo Xie, linux-clk@vger.kernel.org
In-Reply-To: <DB5PR0401MB1928DA88B03970E1EACFB1FF918C0@DB5PR0401MB1928.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: Scott Wood
> Sent: Saturday, March 19, 2016 2:28 AM
> To: Arnd Bergmann; Rob Herring
> Cc: Yangbo Lu; linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org;
> ulf.hansson@linaro.org; Zhao Qiang; Russell King; Bhupesh Sharma;
> netdev@vger.kernel.org; Joerg Roedel; Kumar Gala; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Yang-Leo Li;
> iommu@lists.linux-foundation.org; linux-i2c@vger.kernel.org; Claudiu
> Manoil; Santosh Shilimkar; Xiaobo Xie; linux-clk@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org
> Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
>
> On 03/17/2016 12:06 PM, Arnd Bergmann wrote:
> > On Thursday 17 March 2016 12:01:01 Rob Herring wrote:
> >> On Mon, Mar 14, 2016 at 05:45:43PM +0000, Scott Wood wrote:
> >
> >>>>> This makes the driver non-portable. Better identify the specific
> >>>>> workarounds based on the compatible string for this device, or add
> >>>>> a boolean DT property for the quirk.
> >>>>>
> >>>>> Arnd
> >>>>
> >>>> [Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS
> in v1 before.
> >>>> https://patchwork.kernel.org/patch/6834221/
> >>>>
> >>>> We don't have a separate DTS file for each revision of an SOC and if
> we did, we'd constantly have people using the wrong one.
> >>>> In addition, the device tree is stable ABI and errata are often
> discovered after device tree are deployed.
> >>>> See the link for details.
> >>>>
> >>>> So we decide to read SVR from the device-config/guts MMIO block
> other than using DTS.
> >>>> Thanks.
> >>>
> >>> Also note that this driver is already only for fsl-specific
> >>> hardware, and it will still work even if fsl_guts doesn't find
> >>> anything to bind to
> >>> -- it just wouldn't be able to detect errata based on SVR in that
> case.
> >>
> >> IIRC, it is the same IP block as i.MX and Arnd's point is this won't
> >> even compile on !PPC. It is things like this that prevent sharing the
> >> driver.
>
> The whole point of using the MMIO SVR instead of the PPC SPR is so that
> it will work on ARM... The guts driver should build on any platform as
> long as OF is enabled, and if it doesn't find a node to bind to it will
> return 0 for SVR, and the eSDHC driver will continue (after printing an
> error that should be removed) without the ability to test for errata
> based on SVR.
>
> > I think the first four patches take care of building for ARM, but the
> > problem remains if you want to enable COMPILE_TEST as we need for
> > certain automated checking.
>
> What specific problem is there with COMPILE_TEST?
>
> >> Dealing with Si revs is a common problem. We should have a common
> >> solution. There is soc_device for this purpose.
> >
> > Exactly. The last time this came up, I think we agreed to implement a
> > helper using glob_match() on the soc_device strings. Unfortunately
> > this hasn't happened then, but I'd still prefer that over yet another
> > vendor-specific way of dealing with the generic issue.
>
> soc_device would require encoding the SVR as a string and then decoding
> the string, which is more complicated and error prone than having
> platform-specific code test a platform-specific number. And when would
> it get registered on arm64, which doesn't have platform code?
>
> -Scott
[Lu Yangbo-B47093] Hi Arnd, could you answer Scott's questions?
If you don't oppose this patch, I'd like to rework a new version for merging.
Thanks.
:)
^ permalink raw reply
* RE: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory
From: Yangbo Lu @ 2016-03-25 6:38 UTC (permalink / raw)
To: Scott Wood, Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Zhao Qiang,
Russell King, Bhupesh Sharma,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Santosh Shilimkar,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jochen Friedrich, Xiaobo Xie,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Claudiu Manoil,
Kumar Gala, Yang-Leo Li,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <HE1PR0401MB1931444381E249E41C45C748918C0-B0v07Ae2tarQP5LCsfKN8o3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
> -----Original Message-----
> From: Scott Wood
> Sent: Saturday, March 19, 2016 2:16 AM
> To: Rob Herring; Yangbo Lu
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org;
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; linux-
> clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> foundation.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil;
> ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Bhupesh Sharma; Zhao Qiang; Kumar Gala; Santosh
> Shilimkar; Yang-Leo Li; Xiaobo Xie
> Subject: Re: [v6, 3/5] dt: move guts devicetree doc out of powerpc
> directory
>
> On 03/17/2016 12:06 PM, Rob Herring wrote:
> > On Wed, Mar 09, 2016 at 06:08:49PM +0800, Yangbo Lu wrote:
> >> Move guts devicetree doc to
> >> Documentation/devicetree/bindings/soc/fsl/
> >> since it's used by not only PowerPC but also ARM. And add a
> >> specification for 'little-endian' property.
> >>
> >> Signed-off-by: Yangbo Lu <yangbo.lu-3arQi8VN3Tc@public.gmane.org>
> >> ---
> >> Changes for v2:
> >> - None
> >> Changes for v3:
> >> - None
> >> Changes for v4:
> >> - Added this patch
> >> Changes for v5:
> >> - Modified the description for little-endian property Changes for
> >> v6:
> >> - None
> >> ---
> >> Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt | 3
> >> +++
> >> 1 file changed, 3 insertions(+)
> >> rename Documentation/devicetree/bindings/{powerpc =>
> >> soc}/fsl/guts.txt (91%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> >> b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> >> similarity index 91%
> >> rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> >> rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
> >> index b71b203..07adca9 100644
> >> --- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> >> +++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> >> @@ -25,6 +25,9 @@ Recommended properties:
> >> - fsl,liodn-bits : Indicates the number of defined bits in the LIODN
> >> registers, for those SOCs that have a PAMU device.
> >>
> >> + - little-endian : Indicates that the global utilities block is
> little
> >> + endian. The default is big endian.
> >
> > The default is "the native endianness of the system". So absence on an
> > ARM system would be LE.
>
> No. For this binding, the default is big-endian, because that's what
> existed for this device before an endian property was added.
>
> "endianness of the system" is not a well-defined concept.
>
> > This property is valid for any simple-bus device,
>
> Since when does simple-bus mean anything more than that the nodes
> underneath it can be used without bus-specific knowledge?
>
> > so it isn't really required to document per device. You can, but your
> > description had better match the documented behaviour.
>
> Documented where?
>
> In fact, Documentation/devicetree/bindings/common-properties.txt
> explicitly says of the endian properties, "If a binding supports these
> properties, then the binding should also specify the default behavior if
> none of these properties are present."
>
> -Scott
[Lu Yangbo-B47093] So, Rob, could you accept this patch after so much discussion?
:)
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Cong Wang @ 2016-03-25 5:33 UTC (permalink / raw)
To: Ben Greear; +Cc: Vijay Pandurangan, netdev, Evan Jones, Cong Wang
In-Reply-To: <56F4C8FD.7030907@candelatech.com>
On Thu, Mar 24, 2016 at 10:13 PM, Ben Greear <greearb@candelatech.com> wrote:
>
>
> On 03/24/2016 10:06 PM, Cong Wang wrote:
>>
>> On Thu, Mar 24, 2016 at 9:34 PM, Ben Greear <greearb@candelatech.com>
>> wrote:
>>>
>>>
>>>
>>> On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
>>>>
>>>>
>>>> Oops, I think my last email didn't go through due to an inadvertent
>>>> html attachment from my phone mail client.
>>>>
>>>> Can you send us a copy of a packet you're sending and/or confirm that
>>>> the IP and UDP4 checksums are set correctly in the packet?
>>>>
>>>> If those are set right, I think we need to read through the networking
>>>> code again to see why this is broken...
>>>
>>>
>>>
>>> Wireshark decodes the packet as having no checksum errors.
>>>
>>> I think the contents of the packet is correct, but the 'ip_summed'
>>> field is set incorrectly to 'NONE' when transmitting on a raw packet
>>> socket.
>>
>>
>> Yeah, these bugs are all due to the different interpretations of
>> ip_summed on TX path and RX path. I think the following patch
>> should work, if the comments don't mislead me. Could you give
>> it a try?
>>
>> For the long term, we need to unify the meaning of ip_summed
>> on TX path and RX path, or at least translate it in skb_scrub_packet().
>
>
> I can test this tomorrow, but I think it will not work. I'm not sending raw
> IP frames, I'm sending full ethernet frames. Socket is PF_PACKET, SOCK_RAW.
>
> Your patch may still be useful for others though?
Here we go:
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 1ecfa71..ab66080 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1925,6 +1925,7 @@ static int packet_sendmsg_spkt(struct socket
*sock, struct msghdr *msg,
goto out_unlock;
}
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
skb->protocol = proto;
skb->dev = dev;
skb->priority = sk->sk_priority;
@@ -2496,6 +2497,7 @@ static int tpacket_fill_skb(struct packet_sock
*po, struct sk_buff *skb,
ph.raw = frame;
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
skb->protocol = proto;
skb->dev = dev;
skb->priority = po->sk.sk_priority;
@@ -2805,6 +2807,7 @@ static struct sk_buff *packet_alloc_skb(struct
sock *sk, size_t prepad,
skb_put(skb, linear);
skb->data_len = len - linear;
skb->len += len - linear;
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
return skb;
}
Thanks for testing!
^ permalink raw reply related
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Willy Tarreau @ 2016-03-25 5:28 UTC (permalink / raw)
To: Tom Herbert
Cc: Yann Ylavic, Eric Dumazet, Linux Kernel Network Developers,
Tolga Ceylan, Craig Gallek, Josh Snyder, Aaron Conole,
David S. Miller, Daniel Borkmann
In-Reply-To: <CALx6S37uZah89sNgH9wuD1J+_WEhd34Z5zmrnG8Qp-AQ7Ew=Jg@mail.gmail.com>
On Thu, Mar 24, 2016 at 04:54:03PM -0700, Tom Herbert wrote:
> On Thu, Mar 24, 2016 at 4:40 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
> > I'll learn how to do this to get the best performances from the
> > server, but having to do so to work around what looks like a defect
> > (for simple/default SMP configurations at least, no NUMA or clever
> > CPU-affinity or queuing policy involved) seems odd in the first place.
> >
> I disagree with your assessment that there is a defect. SO_REUSEPORT
> is designed to spread packets amongst _equivalent_ connections. In the
> server draining case sockets are no longer equivalent, but that is a
> special case.
I partially disagree with you here Tom. Initially SO_REUSEPORT was not
used to spread packets but to allow soft restart in some applications.
I've been using it since 2001 in haproxy on *BSD and linux 2.2. It was
removed during 2.3 and I used to keep a patch to reimplement it in 2.4
(basically 2 or 3 lines, the infrastructure was still present), but the
patch was not accepted. The same patch worked for 2.6 and 3.x, allowing
me to continue to perform soft-restarts on Linux just like I used to do
on *BSD. When SO_REUSEPORT was reimplemented in 3.9 with load balancing,
I was happy because it at last allowed me to drop my patch and I got
the extra benefit of better load balancing of incoming connections.
But the main use we have for it (at least historically) is for soft
restarts, where one process replaces another one. Very few people use
more than one process in our case.
However given the benefits of the load spreading for extreme loads,
I'm willing to find how to achieve the same with BPF, but it's pretty
clear that at this point I have no idea where to start from and that
for a single process replacing a single one, it looks quite complicated.
For me quite frankly the special case is the load balancing which is
a side benefit (and a nice one, don't get me wrong).
That's why I would have found it nice to "fix" the process replacement
to avoid dropping incoming connections, though I don't want it to become
a problem for future improvements on BPF. I don't think the two lines I
proposed could become an issue but I'll live without them (or continue
to apply this patch).
BTW, I have no problem with having to write a little bit of assembly for
fast interfaces if it remains untouched for years, we do already have a
bit in haproxy. It's just a longterm investment.
Best regards,
Willy
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Vijay Pandurangan @ 2016-03-25 5:24 UTC (permalink / raw)
To: Ben Greear; +Cc: Cong Wang, netdev, Evan Jones, Cong Wang
In-Reply-To: <56F4C77E.1010608@candelatech.com>
On Fri, Mar 25, 2016 at 1:07 AM, Ben Greear <greearb@candelatech.com> wrote:
> On 03/24/2016 09:45 PM, Vijay Pandurangan wrote:
>>
>> Actually, maybe they should be set to CHECKSUM_PARTIAL if we want veth
>> to drop the packets if they have bad checksums before they hit the
>> application level.
>
>
> VETH is pretty special in that when you transmit a frame on one
> device, it's pair receives it, and unless there is RAM corruption
> or bugs in the kernel, then it cannot be corrupted.
Yeah, you're right that that's an optimization. However, I think that
we should first ensure that
a->veth->b
operates exactly like:
a->physical eth 1 -> physical eth 2->b
in all cases. Once we have that working everywhere we could think
about optimizations.
If we're willing to refactor, we could implement the optimization by
allowing veth devices to know whether their immediate peer is. If a
veth knows it's talking to another veth, it could under some
circumstances elide checksum calculation and verification. I'm not
sure what abstractions that would break, though. What do you guys
think?
>
> But, if you are routing frames from the network to veth
> devices, then the original packet could be corrupted, as
> described in your patch.
>
> Probably the right behaviour is to keep the old logic for packets
> originating from sockets, at least. I am not sure of a good way
> to implement that.
>
> As for testing, I am not sure. Probably you have to make a good
> effort and then just deal with fallout like what I found. I would guess
> that any of us who have ever written an interesting patch have also written
> an interesting bug!
>
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 5:13 UTC (permalink / raw)
To: Cong Wang; +Cc: Vijay Pandurangan, netdev, Evan Jones, Cong Wang
In-Reply-To: <CAM_iQpUaBD-HmHVAcEBCRw38PbecjYk2to+DO9TNkLY93u2_NA@mail.gmail.com>
On 03/24/2016 10:06 PM, Cong Wang wrote:
> On Thu, Mar 24, 2016 at 9:34 PM, Ben Greear <greearb@candelatech.com> wrote:
>>
>>
>> On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
>>>
>>> Oops, I think my last email didn't go through due to an inadvertent
>>> html attachment from my phone mail client.
>>>
>>> Can you send us a copy of a packet you're sending and/or confirm that
>>> the IP and UDP4 checksums are set correctly in the packet?
>>>
>>> If those are set right, I think we need to read through the networking
>>> code again to see why this is broken...
>>
>>
>> Wireshark decodes the packet as having no checksum errors.
>>
>> I think the contents of the packet is correct, but the 'ip_summed'
>> field is set incorrectly to 'NONE' when transmitting on a raw packet
>> socket.
>
> Yeah, these bugs are all due to the different interpretations of
> ip_summed on TX path and RX path. I think the following patch
> should work, if the comments don't mislead me. Could you give
> it a try?
>
> For the long term, we need to unify the meaning of ip_summed
> on TX path and RX path, or at least translate it in skb_scrub_packet().
I can test this tomorrow, but I think it will not work. I'm not sending raw
IP frames, I'm sending full ethernet frames. Socket is PF_PACKET, SOCK_RAW.
Your patch may still be useful for others though?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 5:07 UTC (permalink / raw)
To: Vijay Pandurangan; +Cc: Cong Wang, netdev, Evan Jones, Cong Wang
In-Reply-To: <CAKUBDd8vwvN4z5gP63ukVjRsgbjKQBZvVHv+bJdQRQgcxD5HGA@mail.gmail.com>
On 03/24/2016 09:45 PM, Vijay Pandurangan wrote:
> Actually, maybe they should be set to CHECKSUM_PARTIAL if we want veth
> to drop the packets if they have bad checksums before they hit the
> application level.
VETH is pretty special in that when you transmit a frame on one
device, it's pair receives it, and unless there is RAM corruption
or bugs in the kernel, then it cannot be corrupted.
But, if you are routing frames from the network to veth
devices, then the original packet could be corrupted, as
described in your patch.
Probably the right behaviour is to keep the old logic for packets
originating from sockets, at least. I am not sure of a good way
to implement that.
As for testing, I am not sure. Probably you have to make a good
effort and then just deal with fallout like what I found. I would guess
that any of us who have ever written an interesting patch have also written
an interesting bug!
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Cong Wang @ 2016-03-25 5:06 UTC (permalink / raw)
To: Ben Greear; +Cc: Vijay Pandurangan, netdev, Evan Jones, Cong Wang
In-Reply-To: <56F4BFF1.8010806@candelatech.com>
On Thu, Mar 24, 2016 at 9:34 PM, Ben Greear <greearb@candelatech.com> wrote:
>
>
> On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
>>
>> Oops, I think my last email didn't go through due to an inadvertent
>> html attachment from my phone mail client.
>>
>> Can you send us a copy of a packet you're sending and/or confirm that
>> the IP and UDP4 checksums are set correctly in the packet?
>>
>> If those are set right, I think we need to read through the networking
>> code again to see why this is broken...
>
>
> Wireshark decodes the packet as having no checksum errors.
>
> I think the contents of the packet is correct, but the 'ip_summed'
> field is set incorrectly to 'NONE' when transmitting on a raw packet
> socket.
Yeah, these bugs are all due to the different interpretations of
ip_summed on TX path and RX path. I think the following patch
should work, if the comments don't mislead me. Could you give
it a try?
For the long term, we need to unify the meaning of ip_summed
on TX path and RX path, or at least translate it in skb_scrub_packet().
Thanks.
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 8d22de7..726457e 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -377,7 +377,7 @@ static int raw_send_hdrinc(struct sock *sk, struct
flowi4 *fl4,
iph = ip_hdr(skb);
skb_put(skb, length);
- skb->ip_summed = CHECKSUM_NONE;
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 4319e65..e6b3e31 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -646,7 +646,7 @@ static int rawv6_send_hdrinc(struct sock *sk,
struct msghdr *msg, int length,
skb_reset_network_header(skb);
iph = ipv6_hdr(skb);
- skb->ip_summed = CHECKSUM_NONE;
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
skb->transport_header = skb->network_header;
err = memcpy_from_msg(iph, msg, length);
^ permalink raw reply related
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Vijay Pandurangan @ 2016-03-25 4:45 UTC (permalink / raw)
To: Ben Greear; +Cc: Cong Wang, netdev, Evan Jones, Cong Wang
In-Reply-To: <CAKUBDd8ojaAYHZ6L_8HBetqL1P6v9iy9W9ThZwnnzf3-npt-Vg@mail.gmail.com>
Actually, maybe they should be set to CHECKSUM_PARTIAL if we want veth
to drop the packets if they have bad checksums before they hit the
application level.
On Fri, Mar 25, 2016 at 12:41 AM, Vijay Pandurangan <vijayp@vijayp.ca> wrote:
> agreed. It should maybe be set to CHECKSUM_UNNECESSARY. The comment
> seems to imply that it's treated the same as CHECKSUM_NONE but that's
> evidently not true. I think that would fix the checksumming issue but
> I'm fearful it may break something else:
> http://lxr.free-electrons.com/source/include/linux/skbuff.h#L177
>
> I'm really worried there are other equally subtle bugs hidden in this
> code. Do we have any kind of regression test, or any automated way to
> test all possible values on an skb to determine side effects to any
> change here? (I'm new to the kernel so sorry if there's an answer in
> an FAQ somewhere).
>
> If not,
> 1. how should we ensure that our change doesn't break something else?
> 2. Should we audit / simplify the checksum code or come up with a list
> of test cases that covers all uses?
>
>
> On Fri, Mar 25, 2016 at 12:34 AM, Ben Greear <greearb@candelatech.com> wrote:
>>
>>
>> On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
>>>
>>> Oops, I think my last email didn't go through due to an inadvertent
>>> html attachment from my phone mail client.
>>>
>>> Can you send us a copy of a packet you're sending and/or confirm that
>>> the IP and UDP4 checksums are set correctly in the packet?
>>>
>>> If those are set right, I think we need to read through the networking
>>> code again to see why this is broken...
>>
>>
>> Wireshark decodes the packet as having no checksum errors.
>>
>> I think the contents of the packet is correct, but the 'ip_summed'
>> field is set incorrectly to 'NONE' when transmitting on a raw packet
>> socket.
>>
>>
>> Thanks,
>> Ben
>>
>> --
>> Ben Greear <greearb@candelatech.com>
>> Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Vijay Pandurangan @ 2016-03-25 4:41 UTC (permalink / raw)
To: Ben Greear; +Cc: Cong Wang, netdev, Evan Jones, Cong Wang
In-Reply-To: <56F4BFF1.8010806@candelatech.com>
agreed. It should maybe be set to CHECKSUM_UNNECESSARY. The comment
seems to imply that it's treated the same as CHECKSUM_NONE but that's
evidently not true. I think that would fix the checksumming issue but
I'm fearful it may break something else:
http://lxr.free-electrons.com/source/include/linux/skbuff.h#L177
I'm really worried there are other equally subtle bugs hidden in this
code. Do we have any kind of regression test, or any automated way to
test all possible values on an skb to determine side effects to any
change here? (I'm new to the kernel so sorry if there's an answer in
an FAQ somewhere).
If not,
1. how should we ensure that our change doesn't break something else?
2. Should we audit / simplify the checksum code or come up with a list
of test cases that covers all uses?
On Fri, Mar 25, 2016 at 12:34 AM, Ben Greear <greearb@candelatech.com> wrote:
>
>
> On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
>>
>> Oops, I think my last email didn't go through due to an inadvertent
>> html attachment from my phone mail client.
>>
>> Can you send us a copy of a packet you're sending and/or confirm that
>> the IP and UDP4 checksums are set correctly in the packet?
>>
>> If those are set right, I think we need to read through the networking
>> code again to see why this is broken...
>
>
> Wireshark decodes the packet as having no checksum errors.
>
> I think the contents of the packet is correct, but the 'ip_summed'
> field is set incorrectly to 'NONE' when transmitting on a raw packet
> socket.
>
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 4:34 UTC (permalink / raw)
To: Vijay Pandurangan; +Cc: Cong Wang, netdev, Evan Jones, Cong Wang
In-Reply-To: <CAKUBDd9TjFTvXSEL=+J02PVndNG-gcrMOpRo_MFp4J-oOnLrvQ@mail.gmail.com>
On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
> Oops, I think my last email didn't go through due to an inadvertent
> html attachment from my phone mail client.
>
> Can you send us a copy of a packet you're sending and/or confirm that
> the IP and UDP4 checksums are set correctly in the packet?
>
> If those are set right, I think we need to read through the networking
> code again to see why this is broken...
Wireshark decodes the packet as having no checksum errors.
I think the contents of the packet is correct, but the 'ip_summed'
field is set incorrectly to 'NONE' when transmitting on a raw packet
socket.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [PATCH] net: phy: bcm7xxx: Add entries for Broadcom BCM7346 and BCM7362
From: Jaedon Shin @ 2016-03-25 3:46 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, David S . Miller, Jaedon Shin
Add PHY entries for the Broadcom BCM7346 and BCM7362 chips, these are
40nm generation Ethernet PHY.
Fixes: 815717d1473e ("net: phy: bcm7xxx: Remove wildcard entries")
Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
---
drivers/net/phy/bcm7xxx.c | 4 ++++
include/linux/brcmphy.h | 2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index b881a7b1e4f6..9636da0b6efc 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -339,6 +339,8 @@ static struct phy_driver bcm7xxx_driver[] = {
BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
BCM7XXX_28NM_GPHY(PHY_ID_BCM7439_2, "Broadcom BCM7439 (2)"),
BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
+ BCM7XXX_40NM_EPHY(PHY_ID_BCM7346, "Broadcom BCM7346"),
+ BCM7XXX_40NM_EPHY(PHY_ID_BCM7362, "Broadcom BCM7362"),
BCM7XXX_40NM_EPHY(PHY_ID_BCM7425, "Broadcom BCM7425"),
BCM7XXX_40NM_EPHY(PHY_ID_BCM7429, "Broadcom BCM7429"),
BCM7XXX_40NM_EPHY(PHY_ID_BCM7435, "Broadcom BCM7435"),
@@ -348,6 +350,8 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
{ PHY_ID_BCM7250, 0xfffffff0, },
{ PHY_ID_BCM7364, 0xfffffff0, },
{ PHY_ID_BCM7366, 0xfffffff0, },
+ { PHY_ID_BCM7346, 0xfffffff0, },
+ { PHY_ID_BCM7362, 0xfffffff0, },
{ PHY_ID_BCM7425, 0xfffffff0, },
{ PHY_ID_BCM7429, 0xfffffff0, },
{ PHY_ID_BCM7439, 0xfffffff0, },
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index f0ba9c2ec639..e3354b74286c 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -24,6 +24,8 @@
#define PHY_ID_BCM7250 0xae025280
#define PHY_ID_BCM7364 0xae025260
#define PHY_ID_BCM7366 0x600d8490
+#define PHY_ID_BCM7346 0x600d8650
+#define PHY_ID_BCM7362 0x600d84b0
#define PHY_ID_BCM7425 0x600d86b0
#define PHY_ID_BCM7429 0x600d8730
#define PHY_ID_BCM7435 0x600d8750
--
2.7.4
^ permalink raw reply related
* [PATCH v2] netpoll: Fix extra refcount release in netpoll_cleanup()
From: Bjorn Helgaas @ 2016-03-25 2:56 UTC (permalink / raw)
To: David S. Miller
Cc: Nikolay Aleksandrov, netdev, Neil Horman, Alexander Duyck,
linux-kernel
netpoll_setup() does a dev_hold() on np->dev, the netpoll device. If it
fails, it correctly does a dev_put() but leaves np->dev set. If we call
netpoll_cleanup() after the failure, np->dev is still set so we do another
dev_put(), which decrements the refcount an extra time.
It's questionable to call netpoll_cleanup() after netpoll_setup() fails,
but it can be difficult to find the problem, and we can easily avoid it in
this case. The extra decrements can lead to hangs like this:
unregister_netdevice: waiting for bond0 to become free. Usage count = -3
Set and clear np->dev at the points where we dev_hold() and dev_put() the
device.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
net/core/netpoll.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 94acfc8..a57bd17 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -603,7 +603,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
const struct net_device_ops *ops;
int err;
- np->dev = ndev;
strlcpy(np->dev_name, ndev->name, IFNAMSIZ);
INIT_WORK(&np->cleanup_work, netpoll_async_cleanup);
@@ -670,6 +669,7 @@ int netpoll_setup(struct netpoll *np)
goto unlock;
}
dev_hold(ndev);
+ np->dev = ndev;
if (netdev_master_upper_dev_get(ndev)) {
np_err(np, "%s is a slave device, aborting\n", np->dev_name);
@@ -770,6 +770,7 @@ int netpoll_setup(struct netpoll *np)
return 0;
put:
+ np->dev = NULL;
dev_put(ndev);
unlock:
rtnl_unlock();
^ permalink raw reply related
* [PATCH net-next] phy: keep the BCMR_LOOPBACK bit while setup forced mode
From: Weidong Wang @ 2016-03-25 2:47 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, linux-kernel
When tested the PHY SGMII Loopback,:
1.set the LOOPBACK bit,
2.set the autoneg to AUTONEG_DISABLE, it calls the
genphy_setup_forced which will clear the bit.
So just keep the LOOPBACK bit while setup forced mode.
Signed-off-by: Weidong Wang <wangweidong1@huawei.com>
---
drivers/net/phy/phy_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e551f3a..8da4b80 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1124,7 +1124,9 @@ static int genphy_config_advert(struct phy_device *phydev)
int genphy_setup_forced(struct phy_device *phydev)
{
int ctl = 0;
+ int val = phy_read(phydev, MII_BMCR);
+ ctl |= val & BMCR_LOOPBACK;
phydev->pause = 0;
phydev->asym_pause = 0;
--
2.7.0
^ permalink raw reply related
* [RFC PATCH V2 2/2] vhost: device IOTLB API
From: Jason Wang @ 2016-03-25 2:34 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
Cc: pbonzini, qemu-devel, peterx
In-Reply-To: <1458873274-13961-1-git-send-email-jasowang@redhat.com>
This patch tries to implement an device IOTLB for vhost. This could be
used with for co-operation with userspace(qemu) implementation of DMA
remapping.
The idea is simple. When vhost meets an IOTLB miss, it will request
the assistance of userspace to do the translation, this is done
through:
- Fill the translation request in a preset userspace address (This
address is set through ioctl VHOST_SET_IOTLB_REQUEST_ENTRY).
- Notify userspace through eventfd (This eventfd was set through ioctl
VHOST_SET_IOTLB_FD).
- device IOTLB were started and stopped through VHOST_RUN_IOTLB ioctl
When userspace finishes the translation, it will update the vhost
IOTLB through VHOST_UPDATE_IOTLB ioctl. Userspace is also in charge of
snooping the IOTLB invalidation of IOMMU IOTLB and use
VHOST_UPDATE_IOTLB to invalidate the possible entry in vhost.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 6 +-
drivers/vhost/vhost.c | 301 +++++++++++++++++++++++++++++++++++++++------
drivers/vhost/vhost.h | 17 ++-
fs/eventfd.c | 3 +-
include/uapi/linux/vhost.h | 35 ++++++
5 files changed, 320 insertions(+), 42 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 481db96..7cbdeed 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -334,7 +334,7 @@ static void handle_tx(struct vhost_net *net)
head = vhost_get_vq_desc(vq, vq->iov,
ARRAY_SIZE(vq->iov),
&out, &in,
- NULL, NULL);
+ NULL, NULL, VHOST_ACCESS_RO);
/* On error, stop handling until the next kick. */
if (unlikely(head < 0))
break;
@@ -470,7 +470,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
}
r = vhost_get_vq_desc(vq, vq->iov + seg,
ARRAY_SIZE(vq->iov) - seg, &out,
- &in, log, log_num);
+ &in, log, log_num, VHOST_ACCESS_WO);
if (unlikely(r < 0))
goto err;
@@ -1083,7 +1083,7 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
r = vhost_dev_ioctl(&n->dev, ioctl, argp);
if (r == -ENOIOCTLCMD)
r = vhost_vring_ioctl(&n->dev, ioctl, argp);
- else
+ else if (ioctl != VHOST_UPDATE_IOTLB)
vhost_net_flush(n);
mutex_unlock(&n->dev.mutex);
return r;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 32c35a9..1dd43e8 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -280,6 +280,10 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call_ctx = NULL;
vq->call = NULL;
vq->log_ctx = NULL;
+ vq->iotlb_call = NULL;
+ vq->iotlb_call_ctx = NULL;
+ vq->iotlb_request = NULL;
+ vq->pending_request.flags.type = VHOST_IOTLB_INVALIDATE;
vq->umem = NULL;
vq->is_le = virtio_legacy_is_little_endian();
vhost_vq_reset_user_be(vq);
@@ -387,8 +391,10 @@ void vhost_dev_init(struct vhost_dev *dev,
dev->log_ctx = NULL;
dev->log_file = NULL;
dev->umem = NULL;
+ dev->iotlb = NULL;
dev->mm = NULL;
spin_lock_init(&dev->work_lock);
+ spin_lock_init(&dev->iotlb_lock);
INIT_LIST_HEAD(&dev->work_list);
dev->worker = NULL;
@@ -537,6 +543,15 @@ void vhost_dev_stop(struct vhost_dev *dev)
}
EXPORT_SYMBOL_GPL(vhost_dev_stop);
+static void vhost_umem_free(struct vhost_umem *umem,
+ struct vhost_umem_node *node)
+{
+ vhost_umem_interval_tree_remove(node, &umem->umem_tree);
+ list_del(&node->link);
+ kfree(node);
+ umem->numem--;
+}
+
static void vhost_umem_clean(struct vhost_umem *umem)
{
struct vhost_umem_node *node, *tmp;
@@ -544,11 +559,9 @@ static void vhost_umem_clean(struct vhost_umem *umem)
if (!umem)
return;
- list_for_each_entry_safe(node, tmp, &umem->umem_list, link) {
- vhost_umem_interval_tree_remove(node, &umem->umem_tree);
- list_del(&node->link);
- kvfree(node);
- }
+ list_for_each_entry_safe(node, tmp, &umem->umem_list, link)
+ vhost_umem_free(umem, node);
+
kvfree(umem);
}
@@ -580,6 +593,8 @@ void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
/* No one will access memory at this point */
vhost_umem_clean(dev->umem);
dev->umem = NULL;
+ vhost_umem_clean(dev->iotlb);
+ dev->iotlb = NULL;
WARN_ON(!list_empty(&dev->work_list));
if (dev->worker) {
kthread_stop(dev->worker);
@@ -699,11 +714,61 @@ int vhost_vq_access_ok(struct vhost_virtqueue *vq)
}
EXPORT_SYMBOL_GPL(vhost_vq_access_ok);
+static int vhost_new_umem_range(struct vhost_umem *umem,
+ u64 start, u64 size, u64 end,
+ u64 userspace_addr, int perm)
+{
+ struct vhost_umem_node *tmp, *node = kmalloc(sizeof(*node), GFP_ATOMIC);
+
+ if (!node)
+ return -ENOMEM;
+
+ if (umem->numem == VHOST_IOTLB_SIZE) {
+ tmp = list_last_entry(&umem->umem_list, typeof(*tmp), link);
+ vhost_umem_free(umem, tmp);
+ }
+
+ node->start = start;
+ node->size = size;
+ node->last = end;
+ node->userspace_addr = userspace_addr;
+ node->perm = perm;
+ INIT_LIST_HEAD(&node->link);
+ list_add_tail(&node->link, &umem->umem_list);
+ vhost_umem_interval_tree_insert(node, &umem->umem_tree);
+ umem->numem++;
+
+ return 0;
+}
+
+static void vhost_del_umem_range(struct vhost_umem *umem,
+ u64 start, u64 end)
+{
+ struct vhost_umem_node *node;
+
+ while ((node = vhost_umem_interval_tree_iter_first(&umem->umem_tree,
+ start, end)))
+ vhost_umem_free(umem, node);
+}
+
+static struct vhost_umem *vhost_umem_alloc(void)
+{
+ struct vhost_umem *umem = vhost_kvzalloc(sizeof(*umem));
+
+ if (!umem)
+ return NULL;
+
+ umem->umem_tree = RB_ROOT;
+ umem->numem = 0;
+ INIT_LIST_HEAD(&umem->umem_list);
+
+ return umem;
+}
+
static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
{
struct vhost_memory mem, *newmem;
struct vhost_memory_region *region;
- struct vhost_umem_node *node;
struct vhost_umem *newumem, *oldumem;
unsigned long size = offsetof(struct vhost_memory, regions);
int i;
@@ -725,28 +790,23 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
return -EFAULT;
}
- newumem = vhost_kvzalloc(sizeof(*newumem));
+ newumem = vhost_umem_alloc();
if (!newumem) {
kvfree(newmem);
return -ENOMEM;
}
- newumem->umem_tree = RB_ROOT;
- INIT_LIST_HEAD(&newumem->umem_list);
-
for (region = newmem->regions;
region < newmem->regions + mem.nregions;
region++) {
- node = vhost_kvzalloc(sizeof(*node));
- if (!node)
+ if (vhost_new_umem_range(newumem,
+ region->guest_phys_addr,
+ region->memory_size,
+ region->guest_phys_addr +
+ region->memory_size - 1,
+ region->userspace_addr,
+ VHOST_ACCESS_RW))
goto err;
- node->start = region->guest_phys_addr;
- node->size = region->memory_size;
- node->last = node->start + node->size - 1;
- node->userspace_addr = region->userspace_addr;
- INIT_LIST_HEAD(&node->link);
- list_add_tail(&node->link, &newumem->umem_list);
- vhost_umem_interval_tree_insert(node, &newumem->umem_tree);
}
if (!memory_access_ok(d, newumem, 0))
@@ -782,6 +842,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
struct vhost_vring_state s;
struct vhost_vring_file f;
struct vhost_vring_addr a;
+ struct vhost_vring_iotlb_entry e;
u32 idx;
long r;
@@ -910,6 +971,35 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
} else
filep = eventfp;
break;
+ case VHOST_SET_VRING_IOTLB_REQUEST:
+ r = -EFAULT;
+ if (copy_from_user(&e, argp, sizeof e))
+ break;
+ if (!access_ok(VERIFY_WRITE, e.userspace_addr,
+ sizeof(*vq->iotlb_request)))
+ break;
+ r = 0;
+ vq->iotlb_request = (struct vhost_iotlb_entry __user *)e.userspace_addr;
+ break;
+ case VHOST_SET_VRING_IOTLB_CALL:
+ if (copy_from_user(&f, argp, sizeof f)) {
+ r = -EFAULT;
+ break;
+ }
+ eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
+ if (IS_ERR(eventfp)) {
+ r = PTR_ERR(eventfp);
+ break;
+ }
+ if (eventfp != vq->iotlb_call) {
+ filep = vq->iotlb_call;
+ ctx = vq->iotlb_call_ctx;
+ vq->iotlb_call = eventfp;
+ vq->iotlb_call_ctx = eventfp ?
+ eventfd_ctx_fileget(eventfp) : NULL;
+ } else
+ filep = eventfp;
+ break;
case VHOST_SET_VRING_CALL:
if (copy_from_user(&f, argp, sizeof f)) {
r = -EFAULT;
@@ -977,11 +1067,55 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
}
EXPORT_SYMBOL_GPL(vhost_vring_ioctl);
+static int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled)
+{
+ struct vhost_umem *niotlb, *oiotlb;
+
+ if (enabled) {
+ niotlb = vhost_umem_alloc();
+ if (!niotlb)
+ return -ENOMEM;
+ } else
+ niotlb = NULL;
+
+ spin_lock(&d->iotlb_lock);
+ oiotlb = d->iotlb;
+ d->iotlb = niotlb;
+ spin_unlock(&d->iotlb_lock);
+
+ vhost_umem_clean(oiotlb);
+
+ return 0;
+}
+
+static void vhost_complete_iotlb_update(struct vhost_dev *d,
+ struct vhost_iotlb_entry *entry)
+{
+ struct vhost_iotlb_entry *req;
+ struct vhost_virtqueue *vq;
+ int i;
+
+
+ for (i = 0; i < d->nvqs; i++) {
+ vq = d->vqs[i];
+ mutex_lock(&vq->mutex);
+ req = &vq->pending_request;
+ if (entry->iova <= req->iova &&
+ entry->iova + entry->size - 1 > req->iova &&
+ req->flags.type == VHOST_IOTLB_MISS) {
+ *req = *entry;
+ vhost_poll_queue(&vq->poll);
+ }
+ mutex_unlock(&vq->mutex);
+ }
+}
+
/* Caller must have device mutex */
long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
{
struct file *eventfp, *filep = NULL;
struct eventfd_ctx *ctx = NULL;
+ struct vhost_iotlb_entry entry;
u64 p;
long r;
int i, fd;
@@ -1050,6 +1184,52 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
if (filep)
fput(filep);
break;
+ case VHOST_RUN_IOTLB:
+ /* FIXME: enable and disabled */
+ vhost_init_device_iotlb(d, true);
+ break;
+ case VHOST_UPDATE_IOTLB:
+ r = copy_from_user(&entry, argp, sizeof(entry));
+ if (r < 0) {
+ r = -EFAULT;
+ goto done;
+ }
+
+ spin_lock(&d->iotlb_lock);
+ if (!d->iotlb) {
+ spin_unlock(&d->iotlb_lock);
+ r = -EFAULT;
+ goto done;
+ }
+ switch (entry.flags.type) {
+ case VHOST_IOTLB_UPDATE:
+ if (entry.flags.valid != VHOST_IOTLB_VALID) {
+ break;
+ }
+ if (vhost_new_umem_range(d->iotlb,
+ entry.iova,
+ entry.size,
+ entry.iova + entry.size - 1,
+ entry.userspace_addr,
+ entry.flags.perm)) {
+ r = -ENOMEM;
+ break;
+ }
+ break;
+ case VHOST_IOTLB_INVALIDATE:
+ vhost_del_umem_range(d->iotlb,
+ entry.iova,
+ entry.iova + entry.size - 1);
+ break;
+ default:
+ r = -EINVAL;
+ }
+ spin_unlock(&d->iotlb_lock);
+
+ if (!r && entry.flags.type != VHOST_IOTLB_INVALIDATE)
+ vhost_complete_iotlb_update(d, &entry);
+
+ break;
default:
r = -ENOIOCTLCMD;
break;
@@ -1197,27 +1377,69 @@ int vhost_init_used(struct vhost_virtqueue *vq)
}
EXPORT_SYMBOL_GPL(vhost_init_used);
+static int vhost_iotlb_miss(struct vhost_virtqueue *vq, u64 iova)
+{
+ struct vhost_iotlb_entry *pending = &vq->pending_request;
+ int ret;
+
+ if (!vq->iotlb_call_ctx)
+ return -EOPNOTSUPP;
+
+ if (!vq->iotlb_request)
+ return -EOPNOTSUPP;
+
+ if (pending->flags.type == VHOST_IOTLB_MISS) {
+ return -EEXIST;
+ }
+
+ pending->iova = iova;
+ pending->flags.type = VHOST_IOTLB_MISS;
+
+ ret = __copy_to_user(vq->iotlb_request, pending,
+ sizeof(struct vhost_iotlb_entry));
+ if (ret) {
+ goto err;
+ }
+
+ if (vq->iotlb_call_ctx)
+ eventfd_signal(vq->iotlb_call_ctx, 1);
+err:
+ return ret;
+}
+
static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
- struct iovec iov[], int iov_size)
+ struct iovec iov[], int iov_size, int access)
{
const struct vhost_umem_node *node;
- struct vhost_umem *umem = vq->umem;
+ struct vhost_dev *dev = vq->dev;
+ struct vhost_umem *umem = dev->iotlb ? dev->iotlb : dev->umem;
struct iovec *_iov;
u64 s = 0;
int ret = 0;
+ spin_lock(&dev->iotlb_lock);
+
while ((u64)len > s) {
u64 size;
if (unlikely(ret >= iov_size)) {
ret = -ENOBUFS;
break;
}
+
node = vhost_umem_interval_tree_iter_first(&umem->umem_tree,
addr, addr + len - 1);
if (node == NULL || node->start > addr) {
- ret = -EFAULT;
+ if (umem != dev->iotlb) {
+ ret = -EFAULT;
+ break;
+ }
+ ret = -EAGAIN;
+ break;
+ } else if (!(node->perm & access)) {
+ ret = -EPERM;
break;
}
+
_iov = iov + ret;
size = node->size - addr + node->start;
_iov->iov_len = min((u64)len - s, size);
@@ -1228,6 +1450,10 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
++ret;
}
+ spin_unlock(&dev->iotlb_lock);
+
+ if (ret == -EAGAIN)
+ vhost_iotlb_miss(vq, addr);
return ret;
}
@@ -1256,7 +1482,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
struct iovec iov[], unsigned int iov_size,
unsigned int *out_num, unsigned int *in_num,
struct vhost_log *log, unsigned int *log_num,
- struct vring_desc *indirect)
+ struct vring_desc *indirect, int access)
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
@@ -1274,9 +1500,10 @@ static int get_indirect(struct vhost_virtqueue *vq,
}
ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len, vq->indirect,
- UIO_MAXIOV);
+ UIO_MAXIOV, access);
if (unlikely(ret < 0)) {
- vq_err(vq, "Translation failure %d in indirect.\n", ret);
+ if (ret != -EAGAIN)
+ vq_err(vq, "Translation failure %d in indirect.\n", ret);
return ret;
}
iov_iter_init(&from, READ, vq->indirect, ret, len);
@@ -1316,10 +1543,11 @@ static int get_indirect(struct vhost_virtqueue *vq,
ret = translate_desc(vq, vhost64_to_cpu(vq, desc.addr),
vhost32_to_cpu(vq, desc.len), iov + iov_count,
- iov_size - iov_count);
+ iov_size - iov_count, access);
if (unlikely(ret < 0)) {
- vq_err(vq, "Translation failure %d indirect idx %d\n",
- ret, i);
+ if (ret != -EAGAIN)
+ vq_err(vq, "Translation failure %d indirect idx %d\n",
+ ret, i);
return ret;
}
/* If this is an input descriptor, increment that count. */
@@ -1355,7 +1583,8 @@ static int get_indirect(struct vhost_virtqueue *vq,
int vhost_get_vq_desc(struct vhost_virtqueue *vq,
struct iovec iov[], unsigned int iov_size,
unsigned int *out_num, unsigned int *in_num,
- struct vhost_log *log, unsigned int *log_num)
+ struct vhost_log *log, unsigned int *log_num,
+ int access)
{
struct vring_desc desc;
unsigned int i, head, found = 0;
@@ -1433,10 +1662,11 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_INDIRECT)) {
ret = get_indirect(vq, iov, iov_size,
out_num, in_num,
- log, log_num, &desc);
+ log, log_num, &desc, access);
if (unlikely(ret < 0)) {
- vq_err(vq, "Failure detected "
- "in indirect descriptor at idx %d\n", i);
+ if (ret != -EAGAIN)
+ vq_err(vq, "Failure detected "
+ "in indirect descriptor at idx %d\n", i);
return ret;
}
continue;
@@ -1444,10 +1674,11 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
ret = translate_desc(vq, vhost64_to_cpu(vq, desc.addr),
vhost32_to_cpu(vq, desc.len), iov + iov_count,
- iov_size - iov_count);
+ iov_size - iov_count, access);
if (unlikely(ret < 0)) {
- vq_err(vq, "Translation failure %d descriptor idx %d\n",
- ret, i);
+ if (ret != -EAGAIN)
+ vq_err(vq, "Translation failure %d descriptor idx %d\n",
+ ret, i);
return ret;
}
if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_WRITE)) {
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 5d64393..4365104 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -62,13 +62,15 @@ struct vhost_umem_node {
__u64 last;
__u64 size;
__u64 userspace_addr;
- __u64 flags_padding;
+ __u32 perm;
+ __u32 flags_padding;
__u64 __subtree_last;
};
struct vhost_umem {
struct rb_root umem_tree;
struct list_head umem_list;
+ int numem;
};
/* The virtqueue structure describes a queue attached to a device. */
@@ -84,9 +86,13 @@ struct vhost_virtqueue {
struct file *kick;
struct file *call;
struct file *error;
+ struct file *iotlb_call;
struct eventfd_ctx *call_ctx;
struct eventfd_ctx *error_ctx;
struct eventfd_ctx *log_ctx;
+ struct eventfd_ctx *iotlb_call_ctx;
+ struct vhost_iotlb_entry __user *iotlb_request;
+ struct vhost_iotlb_entry pending_request;
struct vhost_poll poll;
@@ -135,6 +141,8 @@ struct vhost_virtqueue {
#endif
};
+#define VHOST_IOTLB_SIZE 2048
+
struct vhost_dev {
struct mm_struct *mm;
struct mutex mutex;
@@ -146,6 +154,8 @@ struct vhost_dev {
struct list_head work_list;
struct task_struct *worker;
struct vhost_umem *umem;
+ struct vhost_umem *iotlb;
+ spinlock_t iotlb_lock;
};
void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, int nvqs);
@@ -164,7 +174,8 @@ int vhost_log_access_ok(struct vhost_dev *);
int vhost_get_vq_desc(struct vhost_virtqueue *,
struct iovec iov[], unsigned int iov_count,
unsigned int *out_num, unsigned int *in_num,
- struct vhost_log *log, unsigned int *log_num);
+ struct vhost_log *log, unsigned int *log_num,
+ int access);
void vhost_discard_vq_desc(struct vhost_virtqueue *, int n);
int vhost_init_used(struct vhost_virtqueue *);
@@ -183,7 +194,7 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
#define vq_err(vq, fmt, ...) do { \
- pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
+ printk(pr_fmt(fmt), ##__VA_ARGS__); \
if ((vq)->error_ctx) \
eventfd_signal((vq)->error_ctx, 1);\
} while (0)
diff --git a/fs/eventfd.c b/fs/eventfd.c
index 8d0c0df..5c0a22f 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -59,8 +59,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
if (ULLONG_MAX - ctx->count < n)
n = ULLONG_MAX - ctx->count;
ctx->count += n;
- if (waitqueue_active(&ctx->wqh))
+ if (waitqueue_active(&ctx->wqh)) {
wake_up_locked_poll(&ctx->wqh, POLLIN);
+ }
spin_unlock_irqrestore(&ctx->wqh.lock, flags);
return n;
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index ab373191..5c35ab4 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -47,6 +47,32 @@ struct vhost_vring_addr {
__u64 log_guest_addr;
};
+struct vhost_iotlb_entry {
+ __u64 iova;
+ __u64 size;
+ __u64 userspace_addr;
+ struct {
+#define VHOST_ACCESS_RO 0x1
+#define VHOST_ACCESS_WO 0x2
+#define VHOST_ACCESS_RW 0x3
+ __u8 perm;
+#define VHOST_IOTLB_MISS 1
+#define VHOST_IOTLB_UPDATE 2
+#define VHOST_IOTLB_INVALIDATE 3
+ __u8 type;
+#define VHOST_IOTLB_INVALID 0x1
+#define VHOST_IOTLB_VALID 0x2
+ __u8 valid;
+ __u8 u8_padding;
+ __u32 padding;
+ } flags;
+};
+
+struct vhost_vring_iotlb_entry {
+ unsigned int index;
+ __u64 userspace_addr;
+};
+
struct vhost_memory_region {
__u64 guest_phys_addr;
__u64 memory_size; /* bytes */
@@ -127,6 +153,15 @@ struct vhost_memory {
/* Set eventfd to signal an error */
#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
+/* IOTLB */
+/* Specify an eventfd file descriptor to signle on IOTLB miss */
+#define VHOST_SET_VRING_IOTLB_CALL _IOW(VHOST_VIRTIO, 0x23, struct \
+ vhost_vring_file)
+#define VHOST_SET_VRING_IOTLB_REQUEST _IOW(VHOST_VIRTIO, 0x25, struct \
+ vhost_vring_iotlb_entry)
+#define VHOST_UPDATE_IOTLB _IOW(VHOST_VIRTIO, 0x24, struct vhost_iotlb_entry)
+#define VHOST_RUN_IOTLB _IOW(VHOST_VIRTIO, 0x26, int)
+
/* VHOST_NET specific defines */
/* Attach virtio net ring to a raw socket, or tap device.
--
2.5.0
^ permalink raw reply related
* [RFC PATCH V2 1/2] vhost: convert pre sorted vhost memory array to interval tree
From: Jason Wang @ 2016-03-25 2:34 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
Cc: pbonzini, qemu-devel, peterx
In-Reply-To: <1458873274-13961-1-git-send-email-jasowang@redhat.com>
Current pre-sorted memory region array has some limitations for future
device IOTLB conversion:
1) need extra work for adding and removing a single region, and it's
expected to be slow because of sorting or memory re-allocation.
2) need extra work of removing a large range which may intersect
several regions with different size.
3) need trick for a replacement policy like LRU
To overcome the above shortcomings, this patch convert it to interval
tree which can easily address the above issue with almost no extra
work.
The patch could be used for:
- Extend the current API and only let the userspace to send diffs of
memory table.
- Simplify Device IOTLB implementation.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 8 +--
drivers/vhost/vhost.c | 182 ++++++++++++++++++++++++++++----------------------
drivers/vhost/vhost.h | 27 ++++++--
3 files changed, 128 insertions(+), 89 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9eda69e..481db96 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -968,20 +968,20 @@ static long vhost_net_reset_owner(struct vhost_net *n)
struct socket *tx_sock = NULL;
struct socket *rx_sock = NULL;
long err;
- struct vhost_memory *memory;
+ struct vhost_umem *umem;
mutex_lock(&n->dev.mutex);
err = vhost_dev_check_owner(&n->dev);
if (err)
goto done;
- memory = vhost_dev_reset_owner_prepare();
- if (!memory) {
+ umem = vhost_dev_reset_owner_prepare();
+ if (!umem) {
err = -ENOMEM;
goto done;
}
vhost_net_stop(n, &tx_sock, &rx_sock);
vhost_net_flush(n);
- vhost_dev_reset_owner(&n->dev, memory);
+ vhost_dev_reset_owner(&n->dev, umem);
vhost_net_vq_reset(n);
done:
mutex_unlock(&n->dev.mutex);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ad2146a..32c35a9 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -27,6 +27,7 @@
#include <linux/cgroup.h>
#include <linux/module.h>
#include <linux/sort.h>
+#include <linux/interval_tree_generic.h>
#include "vhost.h"
@@ -42,6 +43,10 @@ enum {
#define vhost_used_event(vq) ((__virtio16 __user *)&vq->avail->ring[vq->num])
#define vhost_avail_event(vq) ((__virtio16 __user *)&vq->used->ring[vq->num])
+INTERVAL_TREE_DEFINE(struct vhost_umem_node,
+ rb, __u64, __subtree_last,
+ START, LAST, , vhost_umem_interval_tree);
+
#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY
static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq)
{
@@ -275,7 +280,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call_ctx = NULL;
vq->call = NULL;
vq->log_ctx = NULL;
- vq->memory = NULL;
+ vq->umem = NULL;
vq->is_le = virtio_legacy_is_little_endian();
vhost_vq_reset_user_be(vq);
}
@@ -381,7 +386,7 @@ void vhost_dev_init(struct vhost_dev *dev,
mutex_init(&dev->mutex);
dev->log_ctx = NULL;
dev->log_file = NULL;
- dev->memory = NULL;
+ dev->umem = NULL;
dev->mm = NULL;
spin_lock_init(&dev->work_lock);
INIT_LIST_HEAD(&dev->work_list);
@@ -486,27 +491,36 @@ err_mm:
}
EXPORT_SYMBOL_GPL(vhost_dev_set_owner);
-struct vhost_memory *vhost_dev_reset_owner_prepare(void)
+static void *vhost_kvzalloc(unsigned long size)
+{
+ void *n = kzalloc(size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+
+ if (!n)
+ n = vzalloc(size);
+ return n;
+}
+
+struct vhost_umem *vhost_dev_reset_owner_prepare(void)
{
- return kmalloc(offsetof(struct vhost_memory, regions), GFP_KERNEL);
+ return vhost_kvzalloc(sizeof(struct vhost_umem));
}
EXPORT_SYMBOL_GPL(vhost_dev_reset_owner_prepare);
/* Caller should have device mutex */
-void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_memory *memory)
+void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_umem *umem)
{
int i;
vhost_dev_cleanup(dev, true);
/* Restore memory to default empty mapping. */
- memory->nregions = 0;
- dev->memory = memory;
+ INIT_LIST_HEAD(&umem->umem_list);
+ dev->umem = umem;
/* We don't need VQ locks below since vhost_dev_cleanup makes sure
* VQs aren't running.
*/
for (i = 0; i < dev->nvqs; ++i)
- dev->vqs[i]->memory = memory;
+ dev->vqs[i]->umem = umem;
}
EXPORT_SYMBOL_GPL(vhost_dev_reset_owner);
@@ -523,6 +537,21 @@ void vhost_dev_stop(struct vhost_dev *dev)
}
EXPORT_SYMBOL_GPL(vhost_dev_stop);
+static void vhost_umem_clean(struct vhost_umem *umem)
+{
+ struct vhost_umem_node *node, *tmp;
+
+ if (!umem)
+ return;
+
+ list_for_each_entry_safe(node, tmp, &umem->umem_list, link) {
+ vhost_umem_interval_tree_remove(node, &umem->umem_tree);
+ list_del(&node->link);
+ kvfree(node);
+ }
+ kvfree(umem);
+}
+
/* Caller should have device mutex if and only if locked is set */
void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
{
@@ -549,8 +578,8 @@ void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
fput(dev->log_file);
dev->log_file = NULL;
/* No one will access memory at this point */
- kvfree(dev->memory);
- dev->memory = NULL;
+ vhost_umem_clean(dev->umem);
+ dev->umem = NULL;
WARN_ON(!list_empty(&dev->work_list));
if (dev->worker) {
kthread_stop(dev->worker);
@@ -576,25 +605,25 @@ static int log_access_ok(void __user *log_base, u64 addr, unsigned long sz)
}
/* Caller should have vq mutex and device mutex. */
-static int vq_memory_access_ok(void __user *log_base, struct vhost_memory *mem,
+static int vq_memory_access_ok(void __user *log_base, struct vhost_umem *umem,
int log_all)
{
- int i;
+ struct vhost_umem_node *node;
- if (!mem)
+ if (!umem)
return 0;
- for (i = 0; i < mem->nregions; ++i) {
- struct vhost_memory_region *m = mem->regions + i;
- unsigned long a = m->userspace_addr;
- if (m->memory_size > ULONG_MAX)
+ list_for_each_entry(node, &umem->umem_list, link) {
+ unsigned long a = node->userspace_addr;
+
+ if (node->size > ULONG_MAX)
return 0;
else if (!access_ok(VERIFY_WRITE, (void __user *)a,
- m->memory_size))
+ node->size))
return 0;
else if (log_all && !log_access_ok(log_base,
- m->guest_phys_addr,
- m->memory_size))
+ node->start,
+ node->size))
return 0;
}
return 1;
@@ -602,7 +631,7 @@ static int vq_memory_access_ok(void __user *log_base, struct vhost_memory *mem,
/* Can we switch to this memory table? */
/* Caller should have device mutex but not vq mutex */
-static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
+static int memory_access_ok(struct vhost_dev *d, struct vhost_umem *umem,
int log_all)
{
int i;
@@ -615,7 +644,8 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
log = log_all || vhost_has_feature(d->vqs[i], VHOST_F_LOG_ALL);
/* If ring is inactive, will check when it's enabled. */
if (d->vqs[i]->private_data)
- ok = vq_memory_access_ok(d->vqs[i]->log_base, mem, log);
+ ok = vq_memory_access_ok(d->vqs[i]->log_base,
+ umem, log);
else
ok = 1;
mutex_unlock(&d->vqs[i]->mutex);
@@ -642,7 +672,7 @@ static int vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,
/* Caller should have device mutex but not vq mutex */
int vhost_log_access_ok(struct vhost_dev *dev)
{
- return memory_access_ok(dev, dev->memory, 1);
+ return memory_access_ok(dev, dev->umem, 1);
}
EXPORT_SYMBOL_GPL(vhost_log_access_ok);
@@ -653,7 +683,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq,
{
size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
- return vq_memory_access_ok(log_base, vq->memory,
+ return vq_memory_access_ok(log_base, vq->umem,
vhost_has_feature(vq, VHOST_F_LOG_ALL)) &&
(!vq->log_used || log_access_ok(log_base, vq->log_addr,
sizeof *vq->used +
@@ -669,28 +699,12 @@ int vhost_vq_access_ok(struct vhost_virtqueue *vq)
}
EXPORT_SYMBOL_GPL(vhost_vq_access_ok);
-static int vhost_memory_reg_sort_cmp(const void *p1, const void *p2)
-{
- const struct vhost_memory_region *r1 = p1, *r2 = p2;
- if (r1->guest_phys_addr < r2->guest_phys_addr)
- return 1;
- if (r1->guest_phys_addr > r2->guest_phys_addr)
- return -1;
- return 0;
-}
-
-static void *vhost_kvzalloc(unsigned long size)
-{
- void *n = kzalloc(size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
-
- if (!n)
- n = vzalloc(size);
- return n;
-}
-
static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
{
- struct vhost_memory mem, *newmem, *oldmem;
+ struct vhost_memory mem, *newmem;
+ struct vhost_memory_region *region;
+ struct vhost_umem_node *node;
+ struct vhost_umem *newumem, *oldumem;
unsigned long size = offsetof(struct vhost_memory, regions);
int i;
@@ -710,24 +724,52 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
kvfree(newmem);
return -EFAULT;
}
- sort(newmem->regions, newmem->nregions, sizeof(*newmem->regions),
- vhost_memory_reg_sort_cmp, NULL);
- if (!memory_access_ok(d, newmem, 0)) {
+ newumem = vhost_kvzalloc(sizeof(*newumem));
+ if (!newumem) {
kvfree(newmem);
- return -EFAULT;
+ return -ENOMEM;
+ }
+
+ newumem->umem_tree = RB_ROOT;
+ INIT_LIST_HEAD(&newumem->umem_list);
+
+ for (region = newmem->regions;
+ region < newmem->regions + mem.nregions;
+ region++) {
+ node = vhost_kvzalloc(sizeof(*node));
+ if (!node)
+ goto err;
+ node->start = region->guest_phys_addr;
+ node->size = region->memory_size;
+ node->last = node->start + node->size - 1;
+ node->userspace_addr = region->userspace_addr;
+ INIT_LIST_HEAD(&node->link);
+ list_add_tail(&node->link, &newumem->umem_list);
+ vhost_umem_interval_tree_insert(node, &newumem->umem_tree);
}
- oldmem = d->memory;
- d->memory = newmem;
+
+ if (!memory_access_ok(d, newumem, 0))
+ goto err;
+
+ oldumem = d->umem;
+ d->umem = newumem;
/* All memory accesses are done under some VQ mutex. */
for (i = 0; i < d->nvqs; ++i) {
mutex_lock(&d->vqs[i]->mutex);
- d->vqs[i]->memory = newmem;
+ d->vqs[i]->umem = newumem;
mutex_unlock(&d->vqs[i]->mutex);
}
- kvfree(oldmem);
+
+ kvfree(newmem);
+ vhost_umem_clean(oldumem);
return 0;
+
+err:
+ vhost_umem_clean(newumem);
+ kvfree(newmem);
+ return -EFAULT;
}
long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
@@ -1017,28 +1059,6 @@ done:
}
EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
-static const struct vhost_memory_region *find_region(struct vhost_memory *mem,
- __u64 addr, __u32 len)
-{
- const struct vhost_memory_region *reg;
- int start = 0, end = mem->nregions;
-
- while (start < end) {
- int slot = start + (end - start) / 2;
- reg = mem->regions + slot;
- if (addr >= reg->guest_phys_addr)
- end = slot;
- else
- start = slot + 1;
- }
-
- reg = mem->regions + start;
- if (addr >= reg->guest_phys_addr &&
- reg->guest_phys_addr + reg->memory_size > addr)
- return reg;
- return NULL;
-}
-
/* TODO: This is really inefficient. We need something like get_user()
* (instruction directly accesses the data, with an exception table entry
* returning -EFAULT). See Documentation/x86/exception-tables.txt.
@@ -1180,29 +1200,29 @@ EXPORT_SYMBOL_GPL(vhost_init_used);
static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
struct iovec iov[], int iov_size)
{
- const struct vhost_memory_region *reg;
- struct vhost_memory *mem;
+ const struct vhost_umem_node *node;
+ struct vhost_umem *umem = vq->umem;
struct iovec *_iov;
u64 s = 0;
int ret = 0;
- mem = vq->memory;
while ((u64)len > s) {
u64 size;
if (unlikely(ret >= iov_size)) {
ret = -ENOBUFS;
break;
}
- reg = find_region(mem, addr, len);
- if (unlikely(!reg)) {
+ node = vhost_umem_interval_tree_iter_first(&umem->umem_tree,
+ addr, addr + len - 1);
+ if (node == NULL || node->start > addr) {
ret = -EFAULT;
break;
}
_iov = iov + ret;
- size = reg->memory_size - addr + reg->guest_phys_addr;
+ size = node->size - addr + node->start;
_iov->iov_len = min((u64)len - s, size);
_iov->iov_base = (void __user *)(unsigned long)
- (reg->userspace_addr + addr - reg->guest_phys_addr);
+ (node->userspace_addr + addr - node->start);
s += size;
addr += size;
++ret;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index d3f7674..5d64393 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -52,6 +52,25 @@ struct vhost_log {
u64 len;
};
+#define START(node) ((node)->start)
+#define LAST(node) ((node)->last)
+
+struct vhost_umem_node {
+ struct rb_node rb;
+ struct list_head link;
+ __u64 start;
+ __u64 last;
+ __u64 size;
+ __u64 userspace_addr;
+ __u64 flags_padding;
+ __u64 __subtree_last;
+};
+
+struct vhost_umem {
+ struct rb_root umem_tree;
+ struct list_head umem_list;
+};
+
/* The virtqueue structure describes a queue attached to a device. */
struct vhost_virtqueue {
struct vhost_dev *dev;
@@ -100,7 +119,7 @@ struct vhost_virtqueue {
struct iovec *indirect;
struct vring_used_elem *heads;
/* Protected by virtqueue mutex. */
- struct vhost_memory *memory;
+ struct vhost_umem *umem;
void *private_data;
u64 acked_features;
/* Log write descriptors */
@@ -117,7 +136,6 @@ struct vhost_virtqueue {
};
struct vhost_dev {
- struct vhost_memory *memory;
struct mm_struct *mm;
struct mutex mutex;
struct vhost_virtqueue **vqs;
@@ -127,14 +145,15 @@ struct vhost_dev {
spinlock_t work_lock;
struct list_head work_list;
struct task_struct *worker;
+ struct vhost_umem *umem;
};
void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, int nvqs);
long vhost_dev_set_owner(struct vhost_dev *dev);
bool vhost_dev_has_owner(struct vhost_dev *dev);
long vhost_dev_check_owner(struct vhost_dev *);
-struct vhost_memory *vhost_dev_reset_owner_prepare(void);
-void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_memory *);
+struct vhost_umem *vhost_dev_reset_owner_prepare(void);
+void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_umem *);
void vhost_dev_cleanup(struct vhost_dev *, bool locked);
void vhost_dev_stop(struct vhost_dev *);
long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, void __user *argp);
--
2.5.0
^ permalink raw reply related
* [RFC PATCH V2 0/2] basic device IOTLB support
From: Jason Wang @ 2016-03-25 2:34 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
Cc: pbonzini, qemu-devel, peterx
This patch tries to implement an device IOTLB for vhost. This could be
used with for co-operation with userspace(qemu) implementation of
iommu for a secure DMA environment (DMAR) in guest.
The idea is simple. When vhost meets an IOTLB miss, it will request
the assistance of userspace to do the translation, this is done
through:
- Fill the translation request in a preset userspace address (This
address is set through ioctl VHOST_SET_IOTLB_REQUEST_ENTRY).
- Notify userspace through eventfd (This eventfd was set through ioctl
VHOST_SET_IOTLB_FD).
When userspace finishes the translation, it will update the vhost
IOTLB through VHOST_UPDATE_IOTLB ioctl. Userspace is also in charge of
snooping the IOTLB invalidation of IOMMU IOTLB and use
VHOST_UPDATE_IOTLB to invalidate the possible entry in vhost.
The codes were designed to be architecture independent. It should be
easily ported to any architecture.
Changes from V1:
- support any size/range of updating and invalidation through
introducing the interval tree.
- convert from per device iotlb request to per virtqueue iotlb
request, this solves the possible deadlock in V1.
- read/write permission check support.
Please review.
Thanks
Jason Wang (2):
vhost: convert pre sorted vhost memory array to interval tree
vhost: device IOTLB API
drivers/vhost/net.c | 14 +-
drivers/vhost/vhost.c | 427 +++++++++++++++++++++++++++++++++++----------
drivers/vhost/vhost.h | 42 ++++-
fs/eventfd.c | 3 +-
include/uapi/linux/vhost.h | 35 ++++
5 files changed, 419 insertions(+), 102 deletions(-)
--
2.5.0
^ permalink raw reply
* Re: [PATCH net] net: ipv4: Multipath needs to handle unreachable nexthops
From: David Ahern @ 2016-03-25 2:05 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev
In-Reply-To: <alpine.LFD.2.11.1603242201200.1795@ja.home.ssi.bg>
On 3/24/16 4:33 PM, Julian Anastasov wrote:
> But for multipath routes we can also consider the
> nexthops as "alternatives", so it depends on how one uses
> the multipath mechanism. The ability to fallback to
> another nexthop assumes one connection is allowed to
> move from one ISP to another. What if the second ISP
> decides to reject the connection? What we have is a
> broken connection just because the retransmits
> were diverted to wrong place in the hurry. So, the
> nexthops can be compatible or incompatible. For your
> setup they are, for others they are not.
I am not sure I completely understand your point. Are you saying that
within a single multipath route some connections to nexthops are allowed
and others are not?
So to put that paragraph into an example
15.0.0.0/16
nexthop via 12.0.0.2 dev swp1 weight 1
nexthop via 12.0.0.3 dev swp1 weight 1
Hosts from 15.0/16 could have TCP connections use 12.0.0.2, but not
12.0.0.3 because 12.0.0.3 could be a different ISP and not allow TCP
connections from this address space?
>
> For multipath setups I recall for the following
> possible cases:
>
> 1. Every packet from connection hits multipath route
> and what we want is the connection to use only one
> nexthop. If nexthop fails then the connection fails.
> Latest changes for the multipath algorithm chose this
> option: use hash to map traffic to nexthop and any
> fallbacks to another nexthop are not allowed because
> we should follow the hash mapping. Works when nexthops
> use different ISPs.
>
> 2. Only the first packet hits multipath route. With
> the help from CONNMARK all next packets from connection
> use the same nexthop by using another unicast route.
> The goal here is the multipath route to be used just to
> balance connections. This works best when the nexthop
> selection was random and not a hash based. This was
> how the previous algorithm worked. Fallbacks are
> desired because it is fine to select alive nexthop
> for the first packet, but wrong for the next packets.
>
> My preference was for the random selection,
> I don't know why we restricted the new algorithm. May be
> because in the common case when just a single default
> multipath route is created we want to use all nexthops
> in a ideal world where all nexthops are alive.
>
> So, if the kernel used a random selection
> your fallback algorithm should help. But it is fragile
> for the simple setup with single default multipath route.
> May be what we miss is the ability to choose between
> random and hash-based selection. Then your patch may be
> useful but only for setup 2 (multipath route hit only by
> first packet). So, your patch may come with a sysctl var
> that explains your current patch logic: "avoid failed nexthops,
> never probe them, wait their failed entry to be expired by
> neigh_periodic_work and just then we can use the nexthop
> by creating new entry to probe the GW". Who will trigger
> probes often enough to maintain fresh state?
First packet out does the probe -- neigh lookup fails, kernel has no
knowledge so can't reject the nexthop based on neighbor information.
After that if it has information that says that a nexthop is dead, why
would it continue to try to probe? Any traffic that selects that nh is
dead. That to me defies the basis of having multiple paths.
>
> Also, one may argue that such decisions should
> be done in user space. It is common the direct routers
> to answer ARP even while their uplink is down. In
> the common case, one may need to ping 2-3 indirect
> gateways to decide if a path is alive and then to recreate
> the default multipath route.
>
> More comments below...
>
>> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
>> ---
>> net/ipv4/fib_semantics.c | 34 ++++++++++++++++++++++++++++++++--
>> 1 file changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>> index d97268e8ff10..28fc6700c2b1 100644
>> --- a/net/ipv4/fib_semantics.c
>> +++ b/net/ipv4/fib_semantics.c
>> @@ -1563,13 +1563,43 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
>> void fib_select_multipath(struct fib_result *res, int hash)
>> {
>> struct fib_info *fi = res->fi;
>> + struct neighbour *n;
>> + int state;
>>
>> for_nexthops(fi) {
>> if (hash > atomic_read(&nh->nh_upper_bound))
>> continue;
>>
>> - res->nh_sel = nhsel;
>> - return;
>> + state = NUD_NONE;
>> + n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
>
> Sometimes nh_gw is a local IP, you can call
> neigh_lookup (or a lockless RCU-BH variant) only for the
> nh_scope == RT_SCOPE_LINK case, just like in fib_select_default.
got it, will change.
>
>> + if (n) {
>> + state = n->nud_state;
>> + neigh_release(n);
>> + }
>> + if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
>
> NUD_REACHABLE is part of NUD_VALID, so this is shorter:
>
> if (!n || (state & NUD_VALID)) {
sure.
>
>> + res->nh_sel = nhsel;
>> + return;
>> + }
>> + } endfor_nexthops(fi);
>> +
>> + /* try the nexthops again, but covering the entries
>> + * skipped by the hash
>> + */
>> + fi = res->fi;
>> + for_nexthops(fi) {
>> + if (hash <= atomic_read(&nh->nh_upper_bound))
>
> This is dangerous, we can try a RTNH_F_DEAD entry
> with nh_upper_bound = -1. This can work with 2 checks:
In v2 of my patch I dropped this change as it technically is not needed
for the case I am trying to solve.
>
> if (hash <= atomic_read(&nh->nh_upper_bound))
> break;
> if (atomic_read(&nh->nh_upper_bound) < 0)
> continue;
>
>> + continue;
>> +
>> + state = NUD_NONE;
>> + n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
>> + if (n) {
>> + state = n->nud_state;
>> + neigh_release(n);
>> + }
>> + if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
>> + res->nh_sel = nhsel;
>> + return;
>> + }
>> } endfor_nexthops(fi);
>
> If all are failed why not use the nexthop that was
> selected by the hash? Even if it is failed, new ARP probe
> can succeed.
sure.
Thanks for the comments.
David
^ permalink raw reply
* Re: [RFT Patch net 1/2] ipv6: invalidate the socket cached route on pmtu events if possible
From: Eric Dumazet @ 2016-03-25 1:51 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Wei Wang, Steffen Klassert,
Martin KaFai Lau, Hannes Frederic Sowa, Julian Anastasov
In-Reply-To: <CAM_iQpVDN_LPsa2XgVWoqMjrOYpg9JNjPJVwEvTuxPgYyWRWmg@mail.gmail.com>
On Thu, 2016-03-24 at 17:15 -0700, Cong Wang wrote:
> My understanding is that bh_lock_sock() prevents concurrent
> access to sock struct. Since this is in softirq context, multiple
> CPU's could call this function concurrently, the whole pmtu
> update needs to be done atomically.
>
> UDP, on the other hand, doesn't do this logic, it just looks up
> for dst and save it in sk_dst_cache.
Two ICMP messages processed on two different cpus can still get two
different sockets pointing to the same dst.
I do not see how dst pmtu update could be protected by a lock on each
socket. It would require a dst lock , or some simple memory writes that
do not require special synchro.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox