* Re: [PATCH v8 net-next 1/2] bonding: display xmit_hash_policy for non-dynamic-tlb mode
From: David Miller @ 2014-10-06 21:13 UTC (permalink / raw)
To: maheshb; +Cc: j.vosburgh, andy, vfalico, nikolay, netdev, edumazet, maze
In-Reply-To: <1412469884-27308-1-git-send-email-maheshb@google.com>
From: Mahesh Bandewar <maheshb@google.com>
Date: Sat, 4 Oct 2014 17:44:44 -0700
> It's a trivial fix to display xmit_hash_policy for this new TLB mode
> since it uses transmit-hash-poilicy as part of bonding-master info
> (/proc/net/bonding/<bonding-interface).
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH RFC net 0/2] ipv6: Avoid restarting fib6_lookup() for RTF_CACHE hit
From: David Miller @ 2014-10-06 21:10 UTC (permalink / raw)
To: kafai; +Cc: netdev, hannes
In-Reply-To: <1412374332-26657-1-git-send-email-kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Fri, 3 Oct 2014 15:12:10 -0700
> I am trying to understand why there is a need to restart fib6_lookup() after
> getting rt with RTF_CACHE.
>
> I have adapted the davem's udpflood test
> (https://git.kernel.org/pub/scm/linux/kernel/git/davem/net_test_tools.git) to
> support IPv6 and here is the result:
>
> #root > time ./udpflood -l 20000000 -c 250 2401:db00:face:face::2
>
> Before:
> real 0m33.224s
> user 0m2.941s
> sys 0m30.232s
>
> After:
> real 0m31.517s
> user 0m2.938s
> sys 0m28.536s
If you are serious about seeing these patches integrated, you must
freshly repost this series and provide a proper "Signed-off-by: " tag
for yourself in the commit messages.
^ permalink raw reply
* Re: [Xen-devel] [PATCHv1] xen-netfront: always keep the Rx ring full of requests
From: David Miller @ 2014-10-06 21:07 UTC (permalink / raw)
To: annie.li; +Cc: david.vrabel, netdev, boris.ostrovsky, xen-devel
In-Reply-To: <5432E26C.3020207@oracle.com>
From: annie li <annie.li@oracle.com>
Date: Mon, 06 Oct 2014 14:41:48 -0400
>
> On 2014/10/6 12:00, David Vrabel wrote:
>>>> + queue->rx.req_prod_pvt = req_prod;
>>>> +
>>>> + /* Not enough requests? Try again later. */
>>>> + if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) {
>>>> + mod_timer(&queue->rx_refill_timer, jiffies + (HZ/10));
>>>> + return;
>>> If the previous for loop breaks because of failure of
>>> xennet_alloc_one_rx_buffer, then notify_remote_via_irq is missed here
>>> if
>>> the code returns directly.
>> This is deliberate -- there's no point notifying the backend if there
>> aren't enough requests for the next packet. Since we don't know what
>> the next packet might be we assume it's the largest possible.
> That makes sense.
> However, the largest packet case does not happen so
> frequently. Moreover, netback checks the slots every incoming skb
> requires in xenvif_rx_ring_slots_available, not only concerning the
> largest case.
I have an opinion about the sysfs stuff.
It's user facing, so even if it doesn't influence behavior any more
you have to keep the files around, just make them nops.
^ permalink raw reply
* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Tejun Heo @ 2014-10-06 21:01 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, tiwai, arjan, teg,
rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom
In-Reply-To: <20141006203627.GZ14081@wotan.suse.de>
Hello,
On Mon, Oct 06, 2014 at 10:36:27PM +0200, Luis R. Rodriguez wrote:
> > Do we intend to keep this param permanently? Isn't this more of a
> > temp tool to be used during development? If so, maybe we should make
> > that clear with __DEVEL__ too?
>
> As its designed right now no, its not a temp tool, its there to
> require compatibility with old userspace. For modules we can require
> the module parameter but for built-in we need something else and this
> is what came to mind. It is also what would allow the prefer_async_probe
> flag too as otherwise we won't know if userspace is prepared.
I don't get it. For in-kernel stuff, we already have a clear
synchronization point where we already synchronize all async calls.
Shouldn't we be flushing these async probes there too? insmod'ing is
userland visible but there's no reason this has to be for the built-in
drivers.
Thanks.
--
tejun
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: John Fastabend @ 2014-10-06 20:42 UTC (permalink / raw)
To: Stephen Hemminger
Cc: dborkman, fw, gerlitz.or, hannes, netdev, john.ronciak, amirv,
eric.dumazet, danny.zhou
In-Reply-To: <20141006095553.18097d68@urahara>
On 10/06/2014 09:55 AM, Stephen Hemminger wrote:
> On Sun, 05 Oct 2014 17:06:31 -0700
> John Fastabend <john.fastabend@gmail.com> wrote:
>
>> This patch adds a net_device ops to split off a set of driver queues
>> from the driver and map the queues into user space via mmap. This
>> allows the queues to be directly manipulated from user space. For
>> raw packet interface this removes any overhead from the kernel network
>> stack.
>>
>> Typically in an af_packet interface a packet_type handler is
>> registered and used to filter traffic to the socket and do other
>> things such as fan out traffic to multiple sockets. In this case the
>> networking stack is being bypassed so this code is not run. So the
>> hardware must push the correct traffic to the queues obtained from
>> the ndo callback ndo_split_queue_pairs().
>>
>> Fortunately there is already a flow classification interface which
>> is part of the ethtool command set, ETHTOOL_SRXCLSRLINS. It is
>> currently supported by multiple drivers including sfc, mlx4, niu,
>> ixgbe, and i40e. Supporting some way to steer traffic to a queue
>> is the _only_ hardware requirement to support the interface, plus
>> the driver needs to implement the correct ndo ops. A follow on
>> patch adds support for ixgbe but we expect at least the subset of
>> drivers implementing ETHTOOL_SRXCLSRLINS to be implemented later.
>>
>> The interface is driven over an af_packet socket which we believe
>> is the most natural interface to use. Because it is already used
>> for raw packet interfaces which is what we are providing here.
>> The high level flow for this interface looks like:
>>
>> bind(fd, &sockaddr, sizeof(sockaddr));
>>
>> /* Get the device type and info */
>> getsockopt(fd, SOL_PACKET, PACKET_DEV_DESC_INFO, &def_info,
>> &optlen);
>>
>> /* With device info we can look up descriptor format */
>>
>> /* Get the layout of ring space offset, page_sz, cnt */
>> getsockopt(fd, SOL_PACKET, PACKET_DEV_QPAIR_MAP_REGION_INFO,
>> &info, &optlen);
>>
>> /* request some queues from the driver */
>> setsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
>> &qpairs_info, sizeof(qpairs_info));
>>
>> /* if we let the driver pick us queues learn which queues
>> * we were given
>> */
>> getsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
>> &qpairs_info, sizeof(qpairs_info));
>>
>> /* And mmap queue pairs to user space */
>> mmap(NULL, info.tp_dev_bar_sz, PROT_READ | PROT_WRITE,
>> MAP_SHARED, fd, 0);
>>
>> /* Now we have some user space queues to read/write to*/
>>
>> There is one critical difference when running with these interfaces
>> vs running without them. In the normal case the af_packet module
>> uses a standard descriptor format exported by the af_packet user
>> space headers. In this model because we are working directly with
>> driver queues the descriptor format maps to the descriptor format
>> used by the device. User space applications can learn device
>> information from the socket option PACKET_DEV_DESC_INFO which
>> should provide enough details to extrapulate the descriptor formats.
>> Although this adds some complexity to user space it removes the
>> requirement to copy descriptor fields around.
>>
>> The formats are usually provided by the device vendor documentation
>> If folks want I can provide a follow up patch to provide the formats
>> in a .h file in ./include/uapi/linux/ for ease of use. I have access
>> to formats for ixgbe and mlx drivers other driver owners would need to
>> provide their formats.
>>
>> We tested this interface using traffic generators and doing basic
>> L2 forwarding tests on ixgbe devices. Our tests use a set of patches
>> to DPDK to enable an interface using this socket interfaace. With
>> this interface we can xmit/receive @ line rate from a test user space
>> application on a single core.
>>
>> Additionally we have a set of DPDK patches to enable DPDK with this
>> interface. DPDK can be downloaded @ dpdk.org although as I hope is
>> clear from above DPDK is just our paticular test environment we
>> expect other libraries could be built on this interface.
>>
>> Signed-off-by: Danny Zhou <danny.zhou@intel.com>
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>
> I like the ability to share a device between kernel and user mode networking.
> The model used for DPDK for this is really ugly and fragile/broken.
> Your proposal assumes that you fully trust the user mode networking application
> which is not a generally safe assumption.
>
> A device can DMA from/to any arbitrary physical memory.
> And it would be hard to use IOMMU to protect because the
> IOMMU doesn't know that the difference between the applications queue and
> the rest of the queues.
>
> At least with DPDK you can use VFIO, and you are claiming the whole device to
> allow protection against random memory being read/written.
>
>
However not all platforms support VFIO and when the application
only want to handle specific traffic types a queue maps well to
this.
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Dmitry Torokhov @ 2014-10-06 20:41 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: gregkh, tiwai, tj, arjan, teg, rmilasan, werner, oleg, hare,
bpoirier, santosh, pmladek, dbueso, mcgrof, linux-kernel,
Tetsuo Handa, Joseph Salisbury, Kay Sievers, One Thousand Gnomes,
Tim Gardner, Pierre Fersing, Andrew Morton, Nagalakshmi Nandigama,
Praveen Krishnamoorthy, Sreekanth Reddy, Abhijit Mahajan,
Casey Leedom, Hariprasad S
In-Reply-To: <1412372683-2003-8-git-send-email-mcgrof@do-not-panic.com>
Hi Luis,
On Fri, Oct 03, 2014 at 02:44:43PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> At times we may wish to express the desire to prefer to have
> a device driver probe asynchronously by default. We cannot
> simply enable all device drivers to do this without vetting
> that userspace is prepared for this though given that some
> old userspace is expected to exist which is not equipped to
> deal with broad async probe support. This defines a new kernel
> parameter, bus.enable_kern_async=1, to help address this both to
> help enable async probe support for built-in drivers and to
> enable drivers to specify a preference to opt in for async
> probe support.
>
> If you have a device driver that should use async probe
> support when possible enable the prefer_async_probe bool.
>
> Folks wishing to test enabling async probe for all built-in
> drivers can enable bus.__DEBUG__kernel_force_mod_async_probe=1,
> if you use that though you are on your own.
Thank you for working on this. However there are still couple of issues
with the async probe.
1. As far as I can see you only handle the case when device is already
present and you load a driver. In this case we will do either async or
sync probing, depending on the driver/module settings. However if driver
has already been loaded/registered and we are adding a new device
(another module load, for example you load i2c controller module and it
enumerates its children, or driver signalled deferral during binding)
the probe will be synchronous regardless of the settings.
2. I thin kin the current implementation deferred binding process is
still single-threaded and basically synchronous.
Both of these issues stem form the fact that you only plugging into
bus_add_driver(), but you also need to plug into bus_probe_device(). I
believe I handled these 2 cases properly in the version of patch I sent
a couple of weeks ago so if you could incorporate that in your work that
would be great.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: John Fastabend @ 2014-10-06 20:37 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Daniel Borkmann, John Fastabend, Jesper Dangaard Brouer,
John W. Linville, Neil Horman, Florian Westphal, gerlitz.or,
netdev, john.ronciak, amirv, eric.dumazet, danny.zhou
In-Reply-To: <1412615032.3403.27.camel@localhost>
On 10/06/2014 10:03 AM, Hannes Frederic Sowa wrote:
> Hi John,
>
> On Mo, 2014-10-06 at 08:01 -0700, John Fastabend wrote:
>> On 10/06/2014 02:49 AM, Daniel Borkmann wrote:
>>> Hi John,
>>>
>>> On 10/06/2014 03:12 AM, John Fastabend wrote:
>>>> On 10/05/2014 05:29 PM, Florian Westphal wrote:
>>>>> John Fastabend <john.fastabend@gmail.com> wrote:
>>>>>> There is one critical difference when running with these interfaces
>>>>>> vs running without them. In the normal case the af_packet module
>>>>>> uses a standard descriptor format exported by the af_packet user
>>>>>> space headers. In this model because we are working directly with
>>>>>> driver queues the descriptor format maps to the descriptor format
>>>>>> used by the device. User space applications can learn device
>>>>>> information from the socket option PACKET_DEV_DESC_INFO which
>>>>>> should provide enough details to extrapulate the descriptor formats.
>>>>>> Although this adds some complexity to user space it removes the
>>>>>> requirement to copy descriptor fields around.
>>>>>
>>>>> I find it very disappointing that we seem to have to expose such
>>>>> hardware specific details to userspace via hw-independent interface.
>>>>
>>>> Well it was only for convenience if it doesn't fit as a socket
>>>> option we can remove it. We can look up the device using the netdev
>>>> name from the bind call. I see your point though so if there is
>>>> consensus that this is not needed that is fine.
>>>>
>>>>> How big of a cost are we talking about when you say that it 'removes
>>>>> the requirement to copy descriptor fields'?
>>>>
>>>> This was likely a poor description. If you want to let user space
>>>> poll on the ring (without using system calls or interrupts) then
>>>> I don't see how you can _not_ expose the ring directly complete with
>>>> the vendor descriptor formats.
>>>
>>> But how big is the concrete performance degradation you're seeing if you
>>> use an e.g. `netmap-alike` Linux-own variant as a hw-neutral interface
>>> that does *not* directly expose hw descriptor formats to user space?
>>
>> If we don't directly expose the hardware descriptor formats then we
>> need to somehow kick the driver when we want it to do the copy from
>> the driver descriptor format to the common descriptor format.
>>
>> This requires a system call as far as I can tell. Which has unwanted
>> overhead. I can micro-benchmark this if its helpful. But if we dredge
>> up Jesper's slides here we are really counting cycles so even small
>> numbers count if we want to hit line rate in a user space application
>> with 40Gpbs hardware.
>
> I agree, it seems pretty hard to achieve non-syscall sending on the same
> core, as we somehow must transfer control over to the kernel without
> doing a syscall.
>
> The only other idea would be to export machine code up to user space,
> which you can mmap(MAP_EXEC) from the socket somehow to make this API
> truly NIC agnostic without recompiling. This code then would transform
> the generic descriptors to the hardware specific ones. Seems also pretty
> hairy to do that correctly, maybe.
This seems more complicated then a minimal library in userspace to
load descriptor handling code based on device id.
>
>>> With 1 core netmap does 10G line-rate on 64b; I don't know their numbers
>>> on 40G when run on decent hardware though.
>>>
>>> It would really be great if we have something vendor neutral exposed as
>>> a stable ABI and could leverage emerging infrastructure we already have
>>> in the kernel such as eBPF and recent qdisc batching for raw sockets
>>> instead of reinventing the wheels. (Don't get me wrong, I would love to
>>> see AF_PACKET improved ...)
>>
>> I don't think the interface is vendor specific. It does require some
>> knowledge of the hardware descriptor layout though. It is though vendor
>> neutral from my point of view. I provided the ixgbe patch simple because
>> I'm most familiar with it and have a NIC here. If someone wants to send me
>> a Mellanox NIC I can give it a try although I was hoping to recruit Or or
>> Amir? The only hardware feature required is flow classification to queues
>> which seems to be common across 10Gbps and 40/100Gbps devices. So most
>> of the drivers should be able to support this.
>
> Does flow classification work at the same level as registering network
> addresses? Do I have to bind a e.g. multicast address wie ip maddr and
> then set up flow director/ntuple to get the packets on the correct user
> space facing queue or is it in case of the ixgbe card enough to just add
> those addresses via fdir? Have you thought about letting the
> kernel/driver handle that? In case one would like to connect their
> virtual machines via this interface to the network maybe we need central
> policing and resource constraints for queue management here?
Right now it is enough to program the addresses via fdir. This shouldn't
be ixgbe specific and I did take a quick look at the other drivers use
for fdir and believe it should work the same.
I'm not sure what you mean by kernel/driver handle this? Maybe you mean
from the socket interface? I thought about it briefly but opted for what
I think is more straight forward and using the fdir APIs.
As far as policing and resource constraints I think that is a user space
task. And yes I am working on some user space management applications but
they are still fairly rough.
>
> Do other drivers need a separate af-packet managed way to bind addresses
> to the queue? Maybe there are other quirks we might need to add to
> actually build support for other network interface cards. Would be great
> to at least examine one other driver in regard to this.
I _believe_ this interface is sufficient. I think one of the mellanox
interfaces could be implemented fairly easily.
>
> What other properties of the NIC must be exported? I think we also have
> to deal with MTUs currently configured in the NIC, promisc mode and
> maybe TSO?
We don't support per queue MTUs in the kernel. So I think this can be
learned using existing interfaces.
>
>> If your worried driver writers will implement the interface but not make
>> their descriptor formats easily available I considered putting the layout
>> in a header file in the uapi somewhere. Then we could just reject any
>> implementation that doesn't include the header file needed to use it
>> from user space.
>>
>> With regards to leveraging eBPF and qdisc batching I don't see how this
>> works with direct DMA and polling. Needed to give the lowest overhead
>> between kernel and user space. In this case we want to use the hardware
>> to do the filtering that would normally be done for eBPF and for many
>> use cases the hardware flow classifiers is sufficient.
>
> I agree, those features are hard to connect.
>
>> We already added a qdisc bypass option I see this as taking this path
>> further. I believe there is room for a continuum here. For basic cases
>> use af_packet v1,v2 for mmap rings but using common descriptors use
>> af_packet v3 and set QOS_BYASS. For absolute lowest overhead and
>> specific applications that don't need QOS, eBPF use this interface.
>
> You can simply write C code instead of eBPF code, yes.
>
> I find the six additional ndo ops a bit worrisome as we are adding more
> and more subsystem specific ndoops to this struct. I would like to see
> some unification here, but currently cannot make concrete proposals,
> sorry.
I agree it seems like a bit much. One thought was to split the ndo
ops into categories. Switch ops, MACVLAN ops, basic ops and with this
userspace queue ops. This sort of goes along with some of the switch
offload work which is going to add a handful more ops as best I can
tell.
>
> Patch 2/3 does not yet expose hw ring descriptors in uapi headers it
> seems?
>
Nope I wasn't sure if we wanted to put the ring desciptors in UAPI or
not. If we do I would likely push that as a 4th patch.
> Are there plans to push a user space framework (maybe even into the
> kernel), too? Will this be dpdk (alike) in the end?
I have patches to enable this interface on DPDK and it gets the
same performance as the other DPDK interfaces.
I've considered creating a minimal library to do basic tx/rx and
descriptor processing maybe in ./test or ,/scripts to give a usage
example that is easier to get ahold of and review without having
to pull in all the other things DPDK does that may or may not be
interesting depending on what you are doing and on what hardware.
>
> Bye,
> Hannes
>
>
^ permalink raw reply
* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Luis R. Rodriguez @ 2014-10-06 20:36 UTC (permalink / raw)
To: Tejun Heo
Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, tiwai, arjan, teg,
rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom
In-Reply-To: <20141006201926.GF18303@htj.dyndns.org>
On Mon, Oct 06, 2014 at 04:19:26PM -0400, Tejun Heo wrote:
> Hello, Luis.
>
> The patchset generally looks good to me. Please feel free to add
>
> Reviewed-by: Tejun Heo <tj@kernel.org>
Will do.
> A question below.
>
> On Fri, Oct 03, 2014 at 02:44:43PM -0700, Luis R. Rodriguez wrote:
> > + bus.enable_kern_async=1 [KNL]
> > + This tells the kernel userspace has been vetted for
> > + asynchronous probe support and can listen to the device
> > + driver prefer_async_probe flag for both built-in device
> > + drivers and modules.
>
> Do we intend to keep this param permanently? Isn't this more of a
> temp tool to be used during development? If so, maybe we should make
> that clear with __DEVEL__ too?
As its designed right now no, its not a temp tool, its there to
require compatibility with old userspace. For modules we can require
the module parameter but for built-in we need something else and this
is what came to mind. It is also what would allow the prefer_async_probe
flag too as otherwise we won't know if userspace is prepared.
If we want to get rid of it, it would mean that we're letting go of the idea
that some userspace might exist which depends on *not* doing async probe. As
such I would not consider it a __DEVEL__ param and it'd be a judgement call
to eventually *not require* it. I can see that happening but perhaps a large
series of kernels down the road?
Luis
^ permalink raw reply
* [net-next 2/2] openvswitch: fix a sparse warning
From: Andy Zhou @ 2014-10-06 20:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou
In-Reply-To: <1412626971-30271-1-git-send-email-azhou@nicira.com>
Fix a sparse warning introduced by commit:
f5796684069e0c71c65bce6a6d4766114aec1396 (openvswitch: Add support for
Geneve tunneling.) caught by kbuild test robot:
reproduce:
# apt-get install sparse
# git checkout f5796684069e0c71c65bce6a6d4766114aec1396
# make ARCH=x86_64 allmodconfig
# make C=1 CF=-D__CHECK_ENDIAN__
#
#
# sparse warnings: (new ones prefixed by >>)
#
# >> net/openvswitch/vport-geneve.c:109:15: sparse: incorrect type in assignment (different base types)
# net/openvswitch/vport-geneve.c:109:15: expected restricted __be16 [usertype] sport
# net/openvswitch/vport-geneve.c:109:15: got int
# >> net/openvswitch/vport-geneve.c:110:56: sparse: incorrect type in argument 3 (different base types)
# net/openvswitch/vport-geneve.c:110:56: expected unsigned short [unsigned] [usertype] value
# net/openvswitch/vport-geneve.c:110:56: got restricted __be16 [usertype] sport
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
---
net/openvswitch/vport-geneve.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/openvswitch/vport-geneve.c b/net/openvswitch/vport-geneve.c
index 5572d48..910b3ef 100644
--- a/net/openvswitch/vport-geneve.c
+++ b/net/openvswitch/vport-geneve.c
@@ -104,10 +104,9 @@ static int geneve_get_options(const struct vport *vport,
struct sk_buff *skb)
{
struct geneve_port *geneve_port = geneve_vport(vport);
- __be16 sport;
+ struct inet_sock *sk = inet_sk(geneve_port->gs->sock->sk);
- sport = ntohs(inet_sk(geneve_port->gs->sock->sk)->inet_sport);
- if (nla_put_u16(skb, OVS_TUNNEL_ATTR_DST_PORT, sport))
+ if (nla_put_u16(skb, OVS_TUNNEL_ATTR_DST_PORT, ntohs(sk->inet_sport)))
return -EMSGSIZE;
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [net-next 1/2] net: fix a sparse warning
From: Andy Zhou @ 2014-10-06 20:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou
Fix a sparse warning introduced by Commit
0b5e8b8eeae40bae6ad7c7e91c97c3c0d0e57882 (net: Add Geneve tunneling
protocol driver) caught by kbuild test robot:
# apt-get install sparse
# git checkout 0b5e8b8eeae40bae6ad7c7e91c97c3c0d0e57882
# make ARCH=x86_64 allmodconfig
# make C=1 CF=-D__CHECK_ENDIAN__
#
#
# sparse warnings: (new ones prefixed by >>)
#
# >> net/ipv4/geneve.c:230:42: sparse: incorrect type in assignment (different base types)
# net/ipv4/geneve.c:230:42: expected restricted __be32 [addressable] [assigned] [usertype] s_addr
# net/ipv4/geneve.c:230:42: got unsigned long [unsigned] <noident>
#
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
---
net/ipv4/geneve.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index f008c55..065cd94 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -227,7 +227,7 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
udp_conf.family = AF_INET6;
} else {
udp_conf.family = AF_INET;
- udp_conf.local_ip.s_addr = INADDR_ANY;
+ udp_conf.local_ip.s_addr = htonl(INADDR_ANY);
}
udp_conf.local_udp_port = port;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] net: Add ndo_gso_check
From: Or Gerlitz @ 2014-10-06 20:22 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, John Fastabend, Jeff Kirsher, David Miller,
Linux Netdev List, Thomas Graf, Pravin Shelar, Andy Zhou
In-Reply-To: <CA+mtBx-AW65w7grfb7zTaDKzKQ2n_7JQuLquj1Ayopa4gdiF6Q@mail.gmail.com>
On Mon, Oct 6, 2014 at 8:59 PM, Tom Herbert <therbert@google.com> wrote:
> On Sun, Oct 5, 2014 at 12:13 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> RX wise, Linux tells the driver that UDP port X would be used for
>> VXLAN, right? and indeed, it's possible for some HW implementations
>> not to support RX offloading (checksum) for both VXLAN and NVGRE @ the
>> same time over the same port. But TX/GRO wise, you're probably
>> correct. The thing is that from the user POV they need solution that
>> works for both RX and TX offloading.
> I think from a user POV we want a solution that supports RX and TX
> offloading across the widest range of protocols. This is accomplished
> by implementing protocol agnostic mechanisms like CHECKSUM_COMPLETE
> and protocol agnostic UDP tunnel TSO like we've described. IMO, the
> fact that we have devices that implement protocol specific mechanisms
> for NVGRE and VXLAN should be considered legacy support in the stack,
> for new UDP encapsulation protocols we should not expose specifics in
> the stack in either by adding a GSO type for each protocol, nor
> ndo_add_foo_port for each protocol-- these things will not scale and
> unnecessarily complicate the core stack.
I tend to generally agree to the wind that blows from your writeup, namely:
UDP encapsulation offloads wise, we should pose few general
requirements to NICs to be implemented by vendors in their tomorrow's
HW and treat the current generation (these 4-5 drivers with their
limitations as legacy which should be supported but not state the
stack overall design).
Still we should seek more ways to reduce the pain/amount of
not-well-defined-configurations when these drivers are there and the
stack goes through this upside-down turnaround changes. OTOH you
didn't accept my SKB coloring suggestion for GSO inspection, and OTOH
I guess we can live with some sort of generic helper in the form of
what you suggested, but like it or not, getting rid of
ndo_add_vxlan_port will simply break things out.
Are we going to have a session on the encapsulation/offloads design @ LPC?
I think a replay of your LKS presentation along with open discussion
on how to get there with the legacy requirements could be very
helpful.
Or.
^ permalink raw reply
* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Tejun Heo @ 2014-10-06 20:19 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: gregkh, dmitry.torokhov, tiwai, arjan, teg, rmilasan, werner,
oleg, hare, bpoirier, santosh, pmladek, dbueso, mcgrof,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom, Hariprasad S <harip
In-Reply-To: <1412372683-2003-8-git-send-email-mcgrof@do-not-panic.com>
Hello, Luis.
The patchset generally looks good to me. Please feel free to add
Reviewed-by: Tejun Heo <tj@kernel.org>
A question below.
On Fri, Oct 03, 2014 at 02:44:43PM -0700, Luis R. Rodriguez wrote:
> + bus.enable_kern_async=1 [KNL]
> + This tells the kernel userspace has been vetted for
> + asynchronous probe support and can listen to the device
> + driver prefer_async_probe flag for both built-in device
> + drivers and modules.
Do we intend to keep this param permanently? Isn't this more of a
temp tool to be used during development? If so, maybe we should make
that clear with __DEVEL__ too?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH net-next 0/2] sunvnet: Packet processing in non-interrupt context.
From: David Miller @ 2014-10-06 19:37 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: raghuram.kothakota, netdev
In-Reply-To: <20141006193111.GE24721@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Mon, 6 Oct 2014 15:31:11 -0400
> On (10/06/14 15:25), David Miller wrote:
>>
>> > But we still need to hold the vio lock around the ldc_write
>> > (and also around dring write) in vnet_start_xmit, right?
>>
>> You might be able to avoid it, you're fully serialized by the TX queue
>> lock.
>
> yes, I was just noticing that. The only place where I believe I need
> to hold the vio spin-lock is to sync with the dr->cons checks
> (the "should I send a start_cons LDC message?" check in vnet_start_xmit()
> vs the vnet_ack() updates).
I don't see how that is any different from the netif_queue_wake() checks,
it should be just as easy to cover it with the generic xmit lock.
> But isn't it better in general to declare NETIF_F_LLTX and have finer lock
> granularity in the driver?
No, NETIF_F_LLTX drivers are heavily discouraged. And on the
contrary, it's easier to optimize the locking when we can consolidate
what is covered at both the mid-level of the networking send paths and
what the drivers need in their ->ndo_start_xmit() routines.
^ permalink raw reply
* Re: [PATCH net-next] net: validate_xmit_vlan() is static
From: Joe Perches @ 2014-10-06 19:36 UTC (permalink / raw)
To: Josh Triplett
Cc: Eric Dumazet, David Miller, netdev, Julia Lawall, Dan Carpenter
In-Reply-To: <20141006190728.GA2137@thin>
On Mon, 2014-10-06 at 12:07 -0700, Josh Triplett wrote:
> On Mon, Oct 06, 2014 at 11:44:02AM -0700, Joe Perches wrote:
> > On Mon, 2014-10-06 at 11:26 -0700, Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > Marking this as static allows compiler to inline it.
> >
> > Found by inspection or another tool?
> >
> > Wasn't there some tool to look for non-static functions
> > that are not called externally that could/should be
> > converted to static?
>
> Several: sparse,
Doesn't find anything for this use case
> gcc -Wmissing-prototypes, and findstatic.pl.
Right, thanks, using "make W=1" adds -Wmissing-prototypes.
^ permalink raw reply
* Re: [PATCH net-next] net: validate_xmit_vlan() is static
From: Eric Dumazet @ 2014-10-06 19:35 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, netdev, Julia Lawall, Dan Carpenter, Josh Triplett
In-Reply-To: <1412621042.2916.29.camel@joe-AO725>
On Mon, 2014-10-06 at 11:44 -0700, Joe Perches wrote:
> On Mon, 2014-10-06 at 11:26 -0700, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Marking this as static allows compiler to inline it.
>
> Found by inspection or another tool?
>
> Wasn't there some tool to look for non-static functions
> that are not called externally that could/should be
> converted to static?
Found by inspecting and analyzing performance on real workload.
Note prior commits :
bec3cfdca36bf43cfa3751ad7b56db1a307e0760 net: skb_segment() provides list head and tail
01291202ed4ad548f9a7147d20425cb1d24f49a7 net: do not export skb_gro_receive()
55a93b3ea780908b7d1b3a8cf1976223a9268d78 qdisc: validate skb without holding lock
^ permalink raw reply
* Re: [PATCH net-next 0/2] sunvnet: Packet processing in non-interrupt context.
From: Sowmini Varadhan @ 2014-10-06 19:31 UTC (permalink / raw)
To: David Miller; +Cc: raghuram.kothakota, netdev
In-Reply-To: <20141006.152526.965519223260573233.davem@davemloft.net>
On (10/06/14 15:25), David Miller wrote:
>
> > But we still need to hold the vio lock around the ldc_write
> > (and also around dring write) in vnet_start_xmit, right?
>
> You might be able to avoid it, you're fully serialized by the TX queue
> lock.
yes, I was just noticing that. The only place where I believe I need
to hold the vio spin-lock is to sync with the dr->cons checks
(the "should I send a start_cons LDC message?" check in vnet_start_xmit()
vs the vnet_ack() updates).
But isn't it better in general to declare NETIF_F_LLTX and have finer lock
granularity in the driver?
--Sowmini
^ permalink raw reply
* Re: [PATCH net-next 0/2] sunvnet: Packet processing in non-interrupt context.
From: David Miller @ 2014-10-06 19:25 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: raghuram.kothakota, netdev
In-Reply-To: <20141006160418.GA3604@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Mon, 6 Oct 2014 12:04:18 -0400
>> I think you should be able to get rid of all of the in-driver
>> locking in the fast paths.
>>
>> NAPI ->poll() is non-reentrant, so all RX processing occurs
>> strictly in a serialized environment.
>>
>> Once you do TX reclaim in NAPI context, then all you have to do is
>> take the generic netdev TX queue lock during the evaluation of whether
>> to wakeup the TX queue or not. Worst case you need to hold the
>> TX netdev queue lock across the whole TX reclaim operation.
>>
>> The VIO lock really ought to be entirely superfluous in the data
>> paths.
>
> A few clarifications, since there are more driver-examples using NAPI for
> Rx than for Tx reclaim
Those drivers fully go against our recommendations, we always say that
TX reclaim should also run from NAPI because it liberates SKBs that
therefore become available for RX processing.
> But we still need to hold the vio lock around the ldc_write
> (and also around dring write) in vnet_start_xmit, right?
You might be able to avoid it, you're fully serialized by the TX queue
lock.
^ permalink raw reply
* Re: [PATCH v2 net-next 15/15] tipc: remove old ASCII netlink API
From: David Miller @ 2014-10-06 19:20 UTC (permalink / raw)
To: jon.maloy; +Cc: richard.alpe, netdev, tipc-discussion
In-Reply-To: <A2BAEFC30C8FD34388F02C9B3121859D1C2EB615@eusaamb103.ericsson.se>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Mon, 6 Oct 2014 13:37:14 +0000
> I disagree. We did of course consider this before we posted the patch.
> There is absolutely no reason to believe there are binaries "out there"
> using this API. It is intended for and used only by our own tool "tipc-config",
> and the half-baked documentation that exists about it is not even
> correct. This is an TIPC-internal API/ABI, which has changed over the years,
> and keeps changing. To our best judgement there is no risk in removing the
> old API.
>
> However, we took great care to not break any scripts that might be using
> tipc-config. Therefore, we made this wrapper that provides the old
> tipc-config command API to existing users.
A user should be able to upgrade the kernel and keep using tipc-config
(or _whatever_) that's on their machine.
You cannot break old stuff like this, and just because I didn't notice
you slipping in incompatible changes in the past doesn't mean I'm going
to let you keep doing it.
Backwards compatability must be maintained for every single interface
exposed to the user, and this is non-negotiable, sorry.
^ permalink raw reply
* Re: [PATCH] sctp: handle association restarts when the socket is closed.
From: David Miller @ 2014-10-06 19:18 UTC (permalink / raw)
To: vyasevich; +Cc: netdev
In-Reply-To: <5432943E.7000505@gmail.com>
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Mon, 06 Oct 2014 09:08:14 -0400
> On 10/06/2014 12:22 AM, David Miller wrote:
>> Candidate for -stable?
>
> I say yes. If this problem is triggered it is total pain to get the memory
> clean-up.
Ok, queued up, thanks.
^ permalink raw reply
* Re: [PATCH net-next] ipvs: Avoid null-pointer deref in debug code
From: Julian Anastasov @ 2014-10-06 19:13 UTC (permalink / raw)
To: Alex Gartrell; +Cc: horms, dan.carpenter, lvs-devel, netdev, kernel-team
In-Reply-To: <5432BBB8.9070206@fb.com>
Hello,
On Mon, 6 Oct 2014, Alex Gartrell wrote:
> Hey Julian,
>
> I've sent an updated patch that does this but I have some questions about
> other stuff that I find mildly confusing. Specifically I didn't realize until
> looking at the call sites that !dest || daddr = dest->addr.ip (but maybe I'm
> wrong?)
>
> If that's the case, why do we have the following line in __ip_vs_get_out_rt?
>
> daddr = dest->addr.ip;
Extra line that is not needed...
> If that's /always/ true then we should add the following line or a comment to
> the same effect to clarify
>
> BUG_ON(dest && dest->addr.ip != daddr);
IMHO, BUG_ON is not needed.
> If that's not intended to always be true, then should the patch be the
> following?
>
> ...%pI4", dest ? &dest->addr.ip : &daddr);
Using daddr is fine.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH net-next] net: validate_xmit_vlan() is static
From: Josh Triplett @ 2014-10-06 19:07 UTC (permalink / raw)
To: Joe Perches
Cc: Eric Dumazet, David Miller, netdev, Julia Lawall, Dan Carpenter
In-Reply-To: <1412621042.2916.29.camel@joe-AO725>
On Mon, Oct 06, 2014 at 11:44:02AM -0700, Joe Perches wrote:
> On Mon, 2014-10-06 at 11:26 -0700, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Marking this as static allows compiler to inline it.
>
> Found by inspection or another tool?
>
> Wasn't there some tool to look for non-static functions
> that are not called externally that could/should be
> converted to static?
Several: sparse, gcc -Wmissing-prototypes, and findstatic.pl.
^ permalink raw reply
* Re: [net 0/8] gianfar: ARM port driver updates (1/2)
From: David Miller @ 2014-10-06 19:07 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, Li.Xiubo, Shruti
In-Reply-To: <54324AFE.1040803@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Mon, 6 Oct 2014 10:55:42 +0300
> On 10/6/2014 4:27 AM, David Miller wrote:
>> From: Claudiu Manoil <claudiu.manoil@freescale.com>
>> Date: Fri, 3 Oct 2014 19:02:41 +0300
>>
>>> This is the first round of driver protability fixes and clean-up
>>> with the main purpose to make gianfar portable on ARM, for the ARM
>>> based SoC that integrates the eTSEC ethernet controller - "ls1021a".
>>> The patches primarily address compile time errors, when compiling
>>> gianfar on ARM. They replace PPC specific functions and macros
>>> with architecture independent ones, solve arch specific header
>>> inclusions, guard code that relates to PPC only, and even address
>>> some simple endianess issues (see MAC address setup patch).
>>> The patches addressing the bulk of remaining endianess issues,
>>> like handling DMA fields (BD and FCB), will follow with the sencond
>>> round.
>>> These patches were verified on the ls1021a SoC.
>>
>> If more endianness fixes are necessary and "will follow with the
>> second round", I do not see how you could have verified specifically
>> these changes on the ls1021a.
>>
>
> Hi David,
>
> What I did is to split the initial patchset in 2, to ease up the
> review
> process.
> This first part is fairly straightforward, these patches make
> localized
> code changes and can be more easily ported among different kernel
> versions. The second part has fewer patches but touches more code,
> because it handles endianess conversions for all the reads/writes to
> the buffer descriptors. Please let me now if you have objections to
> this approach.
> As for testing, we have our internal kernel tree for ARM supporting
> ls1021a, and these gianfar patches have been there for a while and
> tested. Now it's time to upstream (a cleaned-up version of) them.
> (see git.freescale.com/git/cgit.cgi/layerscape/ls1021a/linux.git/)
> Please note that the current (upstream) net tree does not include the
> support for ls1021a (which is to be propagated via the arm tree).
I'm merely saying that it's inaccurate to say that you "verified" this
specific patch series on that chip, when in fact the second upcoming
series is necessary as well for the driver to work on that chip properly.
^ permalink raw reply
* Re: [PATCH 08/16] virtio_net: drop config_enable
From: David Miller @ 2014-10-06 19:02 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <1412525038-15871-9-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Sun, 5 Oct 2014 19:07:13 +0300
> Now that virtio core ensures config changes don't arrive during probing,
> drop config_enable flag in virtio net.
> On removal, flush is now sufficient to guarantee that no change work is
> queued.
>
> This help simplify the driver, and will allow setting DRIVER_OK earlier
> without losing config change notifications.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
It's hard for people on the networking side to review these changes
since you haven't CC:'d them on any of the postings necessary to
understand the context of the net/ and drivers/net/ changes.
Please at a minimum CC: everyone on your header [PATCH 0/N] posting
so we know at least at a high level what is going on, and why.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] ipvs: Avoid null-pointer deref in debug code
From: Julian Anastasov @ 2014-10-06 19:01 UTC (permalink / raw)
To: Alex Gartrell; +Cc: horms, dan.carpenter, lvs-devel, netdev, kernel-team
In-Reply-To: <1412610379-6295-1-git-send-email-agartrell@fb.com>
Hello,
On Mon, 6 Oct 2014, Alex Gartrell wrote:
> Use daddr instead of reaching into dest.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Alex Gartrell <agartrell@fb.com>
Thanks!
Acked-by: Julian Anastasov <ja@ssi.bg>
> ---
> net/netfilter/ipvs/ip_vs_xmit.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> index 91f17c1..437a366 100644
> --- a/net/netfilter/ipvs/ip_vs_xmit.c
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -316,7 +316,7 @@ __ip_vs_get_out_rt(int skb_af, struct sk_buff *skb, struct ip_vs_dest *dest,
> if (unlikely(crosses_local_route_boundary(skb_af, skb, rt_mode,
> local))) {
> IP_VS_DBG_RL("We are crossing local and non-local addresses"
> - " daddr=%pI4\n", &dest->addr.ip);
> + " daddr=%pI4\n", &daddr);
> goto err_put;
> }
>
> @@ -458,7 +458,7 @@ __ip_vs_get_out_rt_v6(int skb_af, struct sk_buff *skb, struct ip_vs_dest *dest,
> if (unlikely(crosses_local_route_boundary(skb_af, skb, rt_mode,
> local))) {
> IP_VS_DBG_RL("We are crossing local and non-local addresses"
> - " daddr=%pI6\n", &dest->addr.in6);
> + " daddr=%pI6\n", daddr);
> goto err_put;
> }
>
> --
> Alex Gartrell <agartrell@fb.com>
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH net-next] net: validate_xmit_vlan() is static
From: Joe Perches @ 2014-10-06 18:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Julia Lawall, Dan Carpenter, Josh Triplett
In-Reply-To: <1412619987.11091.76.camel@edumazet-glaptop2.roam.corp.google.com>
On Mon, 2014-10-06 at 11:26 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Marking this as static allows compiler to inline it.
Found by inspection or another tool?
Wasn't there some tool to look for non-static functions
that are not called externally that could/should be
converted to static?
^ 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