* Re: [PATCH v3 net-next 2/2 tuntap: Increase the number of queues in tun.
From: Pankaj Gupta @ 2014-12-10 7:56 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin
Cc: linux-kernel, netdev, davem, dgibson, vfalico, edumazet, vyasevic,
hkchu, wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei shtylyov
In-Reply-To: <5481602E.1030508@redhat.com>
>
> On 12/04/2014 06:20 PM, Michael S. Tsirkin wrote:
> > On Thu, Dec 04, 2014 at 03:03:34AM +0008, Jason Wang wrote:
> >> >
> >> >
> >> > On Wed, Dec 3, 2014 at 5:52 PM, Michael S. Tsirkin <mst@redhat.com>
> >> > wrote:
> >>> > >On Wed, Dec 03, 2014 at 12:49:37PM +0530, Pankaj Gupta wrote:
> >>>> > >> Networking under kvm works best if we allocate a per-vCPU RX and TX
> >>>> > >> queue in a virtual NIC. This requires a per-vCPU queue on the host
> >>>> > >>side.
> >>>> > >> It is now safe to increase the maximum number of queues.
> >>>> > >> Preceding patche: 'net: allow large number of rx queues'
> >>> > >
> >>> > >s/patche/patch/
> >>> > >
> >>>> > >> made sure this won't cause failures due to high order memory
> >>>> > >> allocations. Increase it to 256: this is the max number of vCPUs
> >>>> > >> KVM supports.
> >>>> > >> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >>>> > >> Reviewed-by: David Gibson <dgibson@redhat.com>
> >>> > >
> >>> > >Hmm it's kind of nasty that each tun device is now using x16 memory.
> >>> > >Maybe we should look at using a flex array instead, and removing the
> >>> > >limitation altogether (e.g. make it INT_MAX)?
> >> >
> >> > But this only happens when IFF_MULTIQUEUE were used.
> > I refer to this field:
> > struct tun_file __rcu *tfiles[MAX_TAP_QUEUES];
> > if we make MAX_TAP_QUEUES 256, this will use 4K bytes,
> > apparently unconditionally.
> >
> >
>
> How about just allocate one tfile if IFF_MULTIQUEUE were disabled?
Yes, we can also go with one tfile if for single queue.
As tfiles is array of 'tun_file' pointers with size 256. For multiqueue
we would be using 256 queues. But for single queue if we have one tfile
makes sense.
Also, we are making sure to avoid memory failures with vzalloc.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Antw: Re: Q: need effective backlog for listen()
From: Ulrich Windl @ 2014-12-10 7:23 UTC (permalink / raw)
To: phil; +Cc: netdev
In-Reply-To: <1418137657.384.17.camel@niobium.home.fifi.org>
>>> Philippe Troin <phil@fifi.org> schrieb am 09.12.2014 um 16:07 in Nachricht
<1418137657.384.17.camel@niobium.home.fifi.org>:
> On Tue, 2014-12-09 at 08:01 +0100, Ulrich Windl wrote:
>> >>> Philippe Troin <phil@fifi.org> schrieb am 08.12.2014 um 17:35 in Nachricht
>> <1418056540.384.5.camel@niobium.home.fifi.org>:
>>
>> > The argument to listen() specifies how many connections the system is
>> > allow to keep waiting to be accept()ed.
>> > As soon as you accept() the connection, the count is decremented.
>> > So that won't help for your use case.
>> >
>> >> However none of the above see ms true. Even if my server delays
>> >> accept()ing new connections, no client ever sees a "connection
>> >> refused" or "connection timed out". Is there any chance to signal the
>> >> client that no more connections are accepted at the moment?
>> >
>> > Close the listening socket. No new connections will be accepted.
>> > When you reopen the socket for accepting new connections, you may have
>> > to use SO_REUSEADDR before bind()ing to the port.
>>
>> This is what I had done, but those connections who are waiting to be
>> accepted: If I close the listening socket, will the clients see a
>> connection abort, or will they see a connection refused?
>> Connection aborts could confuse clients.
>
> The clients that completed the 3-way handshake and are waiting for their
> (server-side) socket to be accept()ed will see connection reset I
> believe.
>
> You may be able to work something out by having no listen backlog
> (listen(0)) and closing and reopening the socket when needed.
I think I'd feel happier if listen() would implement the old semantics by default, and a new ioctl() (or such) would change to the "Linux accept all connections (3-way handshake) mode" on demand... Wishful thinking...
Ulrich
^ permalink raw reply
* RE: [PATCH net-next 0/9] r8169:update hardware ephy parameter
From: Hau @ 2014-12-10 7:17 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org
In-Reply-To: <20141209.183549.1930188640715074097.davem@davemloft.net>
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, December 10, 2014 7:36 AM
> To: Hau
> Cc: netdev@vger.kernel.org; nic_swsd; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next 0/9] r8169:update hardware ephy parameter
>
> From: Chunhao Lin <hau@realtek.com>
> Date: Wed, 10 Dec 2014 00:45:54 +0800
>
> > Update hardware ephy parameter to improve pcie compatibility.
>
> This really doesn't tell me anything, I really dislike patch series like this one.
>
> All of the programming is magic values to magic offsets.
>
> You aren't even trying to describe in the commit log message exactly what
> kind of settings are being changed, and exactly how those changes achieve
> the stated goal.
>
> Furthermore, the commit description makes no sense at all to me.
>
> How can programming the ethernet MAC PHY have any influence on PCI-E
> bus compatability? Or are you programming the PCI bus interface's PHY?
>
> In what way are you adjusting which settings and in what way do those
> adjustments help improve PCI-E bus behavior?
>
> You absolutely must describe exactly what the new programming is actually
> doing, precisely, and in detail. I want to know if some kind of timings are
> being adjusted, and in what way.
> Are some fifo limits being changes? If so, in what way, and why does that
> help.
>
> You have to describe what you are doing. Short and non-informative commit
> log messages alongside random changes to undocumented magic constant
> registers is simply unacceptable.
>
These series patch is an alignment with our latest hardware pcie ephy parameters. I will try to explain more on my next patch.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] net: introduce helper macro for_each_cmsghdr
From: Joe Perches @ 2014-12-10 6:56 UTC (permalink / raw)
To: Gu Zheng; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <5487DBD9.1010908@cn.fujitsu.com>
On Wed, 2014-12-10 at 13:36 +0800, Gu Zheng wrote:
> Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
> cmsghdr from msghdr, just cleanup.
This looks nicer.
Ideally this would have used: [PATCH V3] as the subject
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> v2: use the lower-case macro name as Joe suggested.
And a description of the v2->v3 change here.
^ permalink raw reply
* Re: [PATCH v8 3/3] net: hisilicon: new hip04 ethernet driver
From: Ding Tianhong @ 2014-12-10 6:45 UTC (permalink / raw)
To: Arnd Bergmann, Alexander Graf
Cc: Zhangfei Gao, davem, linux, f.fainelli, sergei.shtylyov,
mark.rutland, David.Laight, eric.dumazet, xuwei5,
linux-arm-kernel, netdev, devicetree
In-Reply-To: <5487C357.9030507@huawei.com>
On 2014/12/10 11:51, Ding Tianhong wrote:
> On 2014/12/8 4:09, Arnd Bergmann wrote:
>> On Sunday 07 December 2014 10:49:12 Alexander Graf wrote:
>>> On 07.12.14 04:28, Ding Tianhong wrote:
>>>> On 2014/12/7 8:42, Alexander Graf wrote:
>>>>> On 19.04.14 03:13, Zhangfei Gao wrote:
>>>>>> Support Hisilicon hip04 ethernet driver, including 100M / 1000M controller.
>>>>>> The controller has no tx done interrupt, reclaim xmitted buffer in the poll.
>>>>>>
>>>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>>>
>>>>> Is this driver still supposed to go upstream? I presume this was the
>>>>> last submission and it's been quite some time ago
>>>>>
>>>>
>>>> yes, it is really a long time, but The hip04 did not support tx irq,
>>>> we couldn't get any better idea to fix this defect, do you have any suggestion?
>>>
>>> Well, if hardware doesn't have a TX irq I don't see there's anything we
>>> can do to fix that ;).
>>
>> I don't know if it's related to the ethernet on hip01, but I would assume
>> it is, and that platform is currently being submitted for inclusion, so
>> I'd definitely hope to see this driver get merged too eventually.
>>
>> IIRC, the last revision of the patch set had basically fixed the problem,
>> except for a race that would still allow the napi poll function to exit
>> with poll_complete() but a full queue of TX descriptors and no fallback
>> to clean them up. There was also still an open question about whether or
>> not the driver should use skb_orphan, but I may be misremembering that part.
>>
>
> Hi Arnd:
>
> what about use a state machine to check the tx queue and free the skb, just like:
>
> diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
> index 8593658..71faca8 100644
> --- a/drivers/net/ethernet/hisilicon/hip04_eth.c
> +++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
> @@ -396,9 +396,25 @@ static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> stats->tx_packets++;
> priv->tx_count++;
>
> +
> + queue_delayed_work(priv->wq, &priv->tx_queue, delay);
> +
> return NETDEV_TX_OK;
> }
>
> +static void hip04_tx_queue_monitor(struct work_struct *work)
> +{
> + struct hip04_priv *priv = container_of(work, struct hip04_priv,
> + queue_work.work);
> + struct net_device *dev = priv->ndev;
> + hip04_tx_reclain(ndev, false);
> +
> + if (TX_QUEUE_IS_EMPRY(ndev))
> + return;
> +
> + queue_delayed_work(priv->wq, &priv->tx_queue, delay);
> +}
> +
> static int hip04_rx_poll(struct napi_struct *napi, int budget)
> {
> struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
> @@ -736,6 +752,8 @@ static int hip04_mac_probe(struct platform_device *pdev)
> goto alloc_fail;
> }
>
> + INIT_DELAYED_WORK(&priv->tx_queue, hip04_tx_queue_monitor);
> +
> return 0;
>
>
>
> what do you think of this solution?
>
> Regards
> Ding
>
Miss this code, I think the best way is skb_orphan(skb), just like the cxgb3 drivers, some hardware
didn't use the tx inq to free dmad Tx packages.
Ding
>
>>> Dave, what's your take here? Should we keep a driver from going upstream
>>> just because the hardware is partly broken? I'd really prefer to have an
>>> upstream driver on that SoC rather than some random (eventually even
>>> more broken) downstream code.
>>
>> We can certainly have a slow driver for this hardware, and I'd much
>> prefer slow over broken. I'd guess that some of the performance impact
>> of the missing interrupts can now be offset with the xmit_more logic.
>>
>> Arnd
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> .
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> .
>
^ permalink raw reply
* Re: [PATCHv11 net-next 2/2] openvswitch: Add support for unique flow IDs.
From: Pravin Shelar @ 2014-12-10 6:11 UTC (permalink / raw)
To: Joe Stringer; +Cc: netdev, LKML, dev@openvswitch.org
In-Reply-To: <CANr6G5yOqVkA6hf3J9mk41Kh=0k3sM3tQx4sdADSgFaAx9xrWg@mail.gmail.com>
On Tue, Dec 9, 2014 at 4:25 PM, Joe Stringer <joestringer@nicira.com> wrote:
> On 9 December 2014 at 10:32, Pravin Shelar <pshelar@nicira.com> wrote:
>> On Tue, Dec 2, 2014 at 6:56 PM, Joe Stringer <joestringer@nicira.com> wrote:
>>> @@ -662,11 +664,18 @@ static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stats *stats,
>>> }
>>> }
>>> + error = ovs_nla_copy_ufid(a[OVS_FLOW_ATTR_UFID], &ufid, log);
>>> + if (error)
>>> + return error;
>>> + if (a[OVS_FLOW_ATTR_KEY]) {
>>> + ovs_match_init(&match, &key, &mask);
>>> + error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
>>> + a[OVS_FLOW_ATTR_MASK], log);
>>> + } else if (!ufid) {
>>> OVS_NLERR(log, "Flow key attribute not present in set flow.");
>>> - goto error;
>>> + error = -EINVAL;
>>> }
>>> -
>>> - ovs_match_init(&match, &key, &mask);
>>> - error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
>>> - a[OVS_FLOW_ATTR_MASK], log);
>>> if (error)
>>> goto error;
>>>
>>> - /* Validate actions. */
>>> - if (a[OVS_FLOW_ATTR_ACTIONS]) {
>>> - acts = get_flow_actions(a[OVS_FLOW_ATTR_ACTIONS], &key, &mask,
>>> - log);
>>> - if (IS_ERR(acts)) {
>>> - error = PTR_ERR(acts);
>>> - goto error;
>>> - }
>>> -
>>> - /* Can allocate before locking if have acts. */
>>> - reply = ovs_flow_cmd_alloc_info(acts, info, false);
>>> - if (IS_ERR(reply)) {
>>> - error = PTR_ERR(reply);
>>> - goto err_kfree_acts;
>>> - }
>>> - }
>>> -
>> Why are you moving this action validation under ovs-lock?
>
> The thought was that flow_cmd_set may receive UFID and not key/mask.
> One could imagine a command that sends a UFID and actions, telling OVS
> kmod to update just the actions. Masked key is required for
> ovs_nla_copy_actions(), so in this case a lookup would be required to
> get a masked key.
>
> Perhaps the better alternative for the moment is to still require flow
> key and mask for this command (just as we do for flow_cmd_new). That
> would simplify this change greatly, and doesn't affect current OVS
> userspace.
>
sounds good.
>>> @@ -1194,9 +1254,18 @@ unlock:
>>>
>>> static int ovs_flow_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
>>> {
>>> + struct nlattr *a[__OVS_FLOW_ATTR_MAX];
>>> struct ovs_header *ovs_header = genlmsg_data(nlmsg_data(cb->nlh));
>>> struct table_instance *ti;
>>> struct datapath *dp;
>>> + u32 ufid_flags;
>>> + int err;
>>> +
>>> + err = nlmsg_parse(cb->nlh, GENL_HDRLEN + dp_flow_genl_family.hdrsize,
>>> + a, dp_flow_genl_family.maxattr, flow_policy);
>>
>> Can you add genl helper function for this?
>
> OK.
>
>>> @@ -1235,6 +1304,8 @@ static const struct nla_policy flow_policy[OVS_FLOW_ATTR_MAX + 1] = {
>>> [OVS_FLOW_ATTR_ACTIONS] = { .type = NLA_NESTED },
>>> [OVS_FLOW_ATTR_CLEAR] = { .type = NLA_FLAG },
>>> [OVS_FLOW_ATTR_PROBE] = { .type = NLA_FLAG },
>>> + [OVS_FLOW_ATTR_UFID] = { .type = NLA_UNSPEC },
>>> + [OVS_FLOW_ATTR_UFID_FLAGS] = { .type = NLA_U32 },
>>> };
>>>
>>> static const struct genl_ops dp_flow_genl_ops[] = {
>>> diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
>>> index a8b30f3..7f31dbf 100644
>>> --- a/net/openvswitch/flow.h
>>> +++ b/net/openvswitch/flow.h
>>> @@ -197,6 +197,13 @@ struct sw_flow_match {
>>> struct sw_flow_mask *mask;
>>> };
>>>
>>> +#define MAX_UFID_LENGTH 256
>>> +
>> For now we can limit this to 128 bits.
>
> Is there a reason beyond trying to avoid the warning in flow_cmd_set()?
> I suppose that its purpose as an identifier means that it's unlikely to
> need to be much bigger in future (indeed, the larger it gets, the more
> it would trade off the performance gains from using it).
>
I am also not sure why would we need ID larger than 128 bit. In such
unlikely scenario I think we can increase it later if we need it.
>>> @@ -213,13 +220,16 @@ struct flow_stats {
>>>
>>> struct sw_flow {
>>> struct rcu_head rcu;
>>> - struct hlist_node hash_node[2];
>>> - u32 hash;
>>> + struct {
>>> + struct hlist_node node[2];
>>> + u32 hash;
>>> + } flow_hash, ufid_hash;
>> I am not sure about _hash suffix here, Can you explain it? I think
>> _table is better.
>
> Agreed, table is better.
>
>>> int stats_last_writer; /* NUMA-node id of the last writer on
>>> * 'stats[0]'.
>>> */
>>> struct sw_flow_key key;
>>> - struct sw_flow_key unmasked_key;
>>> + struct sw_flow_id *ufid;
>> Lets move this structure inside sw_flow, so that we can avoid one
>> kmalloc during flow-install in case of UFID. something like:
>>
>> struct {
>> u32 ufid_len;
>> union {
>> u32 ufid[MAX_UFID_LENGTH / 4];
>> struct sw_flow_key *unmasked_key;
>> }
>> } id;
>
> Agreed.
>
>>> @@ -424,10 +475,9 @@ static struct sw_flow *masked_flow_lookup(struct table_instance *ti,
>>> ovs_flow_mask_key(&masked_key, unmasked, mask);
>>> hash = flow_hash(&masked_key, key_start, key_end);
>>> head = find_bucket(ti, hash);
>>> - hlist_for_each_entry_rcu(flow, head, hash_node[ti->node_ver]) {
>>> - if (flow->mask == mask && flow->hash == hash &&
>>> - flow_cmp_masked_key(flow, &masked_key,
>>> - key_start, key_end))
>>> + hlist_for_each_entry_rcu(flow, head, flow_hash.node[ti->node_ver]) {
>>> + if (flow->mask == mask && flow->flow_hash.hash == hash &&
>>> + flow_cmp_masked_key(flow, &masked_key, key_start, key_end))
>>> return flow;
>>> }
>>> return NULL;
>>> @@ -469,7 +519,40 @@ struct sw_flow *ovs_flow_tbl_lookup_exact(struct flow_table *tbl,
>>> /* Always called under ovs-mutex. */
>>> list_for_each_entry(mask, &tbl->mask_list, list) {
>>> flow = masked_flow_lookup(ti, match->key, mask);
>>> - if (flow && ovs_flow_cmp_unmasked_key(flow, match)) /* Found */
>>> + if (flow && !flow->ufid &&
>> why not NULL check for flow->unmasked_key here rather than ufid?
>
> In this version, I tried to consistently use flow->ufid as the switch
> for whether UFID exists or not. In the next version, this statement
> would refer to flow->id->ufid_len.
>
> The current approach means that ovs_flow_tbl_lookup_exact() is really
> ovs_flow_tbl_lookup_unmasked_key(). Do you think this should remain
> specific to unmasked key or should it be made to check that the
> identifier (ufid OR unmasked key) is the same?
It is easier to read code if we check for flow->unmasked_key here.
ovs_flow_cmp_unmasked_key() has assert on ufid anyways.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: fix sparse warning
From: Ying Xue @ 2014-12-10 5:54 UTC (permalink / raw)
To: Eric Dumazet, davem; +Cc: netdev
In-Reply-To: <1418190018.27198.12.camel@edumazet-glaptop2.roam.corp.google.com>
On 12/10/2014 01:40 PM, Eric Dumazet wrote:
> On Wed, 2014-12-10 at 11:28 +0800, Ying Xue wrote:
>> This fixes the following sparse warning when using
>>
>> make C=1 CF=-D__CHECK_ENDIAN__ net/ipv6/addrconf.o
>> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
>> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
>> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
>> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
>>
>> To silence above spare complaint, an RCU annotation should be added
>> to next pointer of hlist_node structure through hlist_next_rcu() macro
>> when iterating over a hlist with hlist_for_each_entry_continue_rcu_bh().
>>
>> By the way, this commit also resolves the same error appearing in
>> hlist_for_each_entry_continue_rcu().
>
> This probably should be sent to lkml and CC Paul E. McKenney
>
>
>
>
I prefer that the patch as well as another similar patch ("tipc: fix RCU
sparse error") are still based on net-next tree, but I will CC Paul E.
McKenney and lkml when resend them, is it ok?
Especially for the patch ("tipc: fix RCU sparse error"), it fixes the
sparse error introduced by the commit 97ede29e80ee ("tipc: convert name
table read-write lock to RCU") which was only merged into net-next tree.
Regards,
Ying
^ permalink raw reply
* ipv6: oops in datagram.c line 260
From: Chris Ruehl @ 2014-12-10 5:43 UTC (permalink / raw)
To: netdev
Hi all,
We running a Dell server which crash frequently with (dell crash video
snapshot) vanilla 3.14.25
Capture viewed here: http://www.gtsys.com.hk/~chris/datagram_c_line260.png
The capture don't sadly don't show the full trace, so we lack on
information.
1st line I can see in the crash video from the idrac :
tcp_transmit_skb+0x461
RIP [<ffffffff815da587>] ipv6_local_error+0x17/0x140
The null pointer happen:
Type "apropos word" to search for commands related to "word"...
Reading symbols from net/ipv6/datagram.o...done.
(gdb) list *(ipv6_local_error+0x17)
0xae7 is in ipv6_local_error (net/ipv6/datagram.c:260).
255 struct ipv6_pinfo *np = inet6_sk(sk);
256 struct sock_exterr_skb *serr;
257 struct ipv6hdr *iph;
258 struct sk_buff *skb;
259
260 if (!np->recverr)
261 return;
262
263 skb = alloc_skb(sizeof(struct ipv6hdr), GFP_ATOMIC);
264 if (!skb)
(gdb) quit
We running a 6in4 with ipsec tunnel on the 6. I found a pull request from
Steffen Klassert
here:
http://article.gmane.org/gmane.linux.network/281469
Which might be relevant to this problem.
For time being I add a
if (np == NULL){
LIMIT_NETDEBUG(KERN_DEBUG "ipv6_pinfo is NULL\n");
return;
}
as work around to stop the server crashing
With kind regards
Chris
^ permalink raw reply
* [PATCH v2] if_bridge: fix conflict with glibc
From: Stephen Hemminger @ 2014-12-10 5:41 UTC (permalink / raw)
To: David Miller; +Cc: gregory.0xf0, f.fainelli, xiyou.wangcong, netdev
In-Reply-To: <20141209.234954.1634249639621060084.davem@davemloft.net>
Even though kernel and glibc headers have same effective values
Gcc complains about redefinitions. Since this is a header expected
to be used by userspace, use glibc header.
This supersedes change in commit 66f1c44887ba4f47d617f8ae21cf8e04e1892bd7
and fixes build of iproute2 with Glibc-2.19. This follows similar usage
in include/uapi/linux for l2tp.h
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
--- a/include/uapi/linux/if_bridge.h 2014-12-09 20:25:21.656016605 -0800
+++ b/include/uapi/linux/if_bridge.h 2014-12-09 21:32:01.391034756 -0800
@@ -15,7 +15,9 @@
#include <linux/types.h>
#include <linux/if_ether.h>
-#include <linux/in6.h>
+#ifndef __KERNEL__
+#include <netinet/in.h>
+#endif
#define SYSFS_BRIDGE_ATTR "bridge"
#define SYSFS_BRIDGE_FDB "brforward"
^ permalink raw reply
* Re: [PATCH net-next] ipv6: fix sparse warning
From: Eric Dumazet @ 2014-12-10 5:40 UTC (permalink / raw)
To: Ying Xue; +Cc: davem, netdev
In-Reply-To: <1418182101-7992-1-git-send-email-ying.xue@windriver.com>
On Wed, 2014-12-10 at 11:28 +0800, Ying Xue wrote:
> This fixes the following sparse warning when using
>
> make C=1 CF=-D__CHECK_ENDIAN__ net/ipv6/addrconf.o
> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
>
> To silence above spare complaint, an RCU annotation should be added
> to next pointer of hlist_node structure through hlist_next_rcu() macro
> when iterating over a hlist with hlist_for_each_entry_continue_rcu_bh().
>
> By the way, this commit also resolves the same error appearing in
> hlist_for_each_entry_continue_rcu().
This probably should be sent to lkml and CC Paul E. McKenney
^ permalink raw reply
* [PATCH v2] net: introduce helper macro for_each_cmsghdr
From: Gu Zheng @ 2014-12-10 5:36 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, Joe Perches
Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v2: use the lower-case macro name as Joe suggested.
---
.../networking/timestamping/timestamping.c | 4 +---
.../networking/timestamping/txtimestamp.c | 4 +---
crypto/af_alg.c | 2 +-
include/linux/socket.h | 4 ++++
net/core/scm.c | 3 +--
net/dccp/proto.c | 5 ++---
net/ipv4/ip_sockglue.c | 2 +-
net/ipv6/datagram.c | 2 +-
net/iucv/af_iucv.c | 4 +---
net/rds/send.c | 4 ++--
net/rxrpc/ar-output.c | 2 +-
net/sctp/socket.c | 3 +--
12 files changed, 17 insertions(+), 22 deletions(-)
diff --git a/Documentation/networking/timestamping/timestamping.c b/Documentation/networking/timestamping/timestamping.c
index 5cdfd74..635a43a 100644
--- a/Documentation/networking/timestamping/timestamping.c
+++ b/Documentation/networking/timestamping/timestamping.c
@@ -169,9 +169,7 @@ static void printpacket(struct msghdr *msg, int res,
res,
inet_ntoa(from_addr->sin_addr),
msg->msg_controllen);
- for (cmsg = CMSG_FIRSTHDR(msg);
- cmsg;
- cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
printf(" cmsg len %zu: ", cmsg->cmsg_len);
switch (cmsg->cmsg_level) {
case SOL_SOCKET:
diff --git a/Documentation/networking/timestamping/txtimestamp.c b/Documentation/networking/timestamping/txtimestamp.c
index b32fc2a..f4fd9a2 100644
--- a/Documentation/networking/timestamping/txtimestamp.c
+++ b/Documentation/networking/timestamping/txtimestamp.c
@@ -149,9 +149,7 @@ static void __recv_errmsg_cmsg(struct msghdr *msg, int payload_len)
struct scm_timestamping *tss = NULL;
struct cmsghdr *cm;
- for (cm = CMSG_FIRSTHDR(msg);
- cm && cm->cmsg_len;
- cm = CMSG_NXTHDR(msg, cm)) {
+ for_each_cmsghdr(cmsg, msg) {
if (cm->cmsg_level == SOL_SOCKET &&
cm->cmsg_type == SCM_TIMESTAMPING) {
tss = (void *) CMSG_DATA(cm);
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 6a3ad80..bc21f52 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -399,7 +399,7 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con)
{
struct cmsghdr *cmsg;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
if (cmsg->cmsg_level != SOL_ALG)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index bb9b836..0e71278 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -94,6 +94,10 @@ struct cmsghdr {
(cmsg)->cmsg_len <= (unsigned long) \
((mhdr)->msg_controllen - \
((char *)(cmsg) - (char *)(mhdr)->msg_control)))
+#define for_each_cmsghdr(cmsg, msg) \
+ for (cmsg = CMSG_FIRSTHDR(msg); \
+ cmsg; \
+ cmsg = CMSG_NXTHDR(msg, cmsg))
/*
* Get the next cmsg header
diff --git a/net/core/scm.c b/net/core/scm.c
index b442e7e..3b6899b 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -129,8 +129,7 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
struct cmsghdr *cmsg;
int err;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg))
- {
+ for_each_cmsghdr(cmsg, msg) {
err = -EINVAL;
/* Verify that cmsg_len is at least sizeof(struct cmsghdr) */
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 5ab6627..e7413a9 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -703,7 +703,7 @@ EXPORT_SYMBOL_GPL(compat_dccp_getsockopt);
static int dccp_msghdr_parse(struct msghdr *msg, struct sk_buff *skb)
{
- struct cmsghdr *cmsg = CMSG_FIRSTHDR(msg);
+ struct cmsghdr *cmsg;
/*
* Assign an (opaque) qpolicy priority value to skb->priority.
@@ -717,8 +717,7 @@ static int dccp_msghdr_parse(struct msghdr *msg, struct sk_buff *skb)
*/
skb->priority = 0;
- for (; cmsg != NULL; cmsg = CMSG_NXTHDR(msg, cmsg)) {
-
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 9daf217..839db9d 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -192,7 +192,7 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc,
int err, val;
struct cmsghdr *cmsg;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 2cdc383..7252965 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -640,7 +640,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
int len;
int err = 0;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
int addr_type;
if (!CMSG_OK(msg, cmsg)) {
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index a089b6b..b69d87e 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1070,9 +1070,7 @@ static int iucv_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
txmsg.class = 0;
/* iterate over control messages */
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg;
- cmsg = CMSG_NXTHDR(msg, cmsg)) {
-
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg)) {
err = -EINVAL;
goto out;
diff --git a/net/rds/send.c b/net/rds/send.c
index 0a64541..b23e7b8 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -826,7 +826,7 @@ static int rds_rm_size(struct msghdr *msg, int data_len)
int cmsg_groups = 0;
int retval;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
@@ -878,7 +878,7 @@ static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm,
struct cmsghdr *cmsg;
int ret = 0;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
diff --git a/net/rxrpc/ar-output.c b/net/rxrpc/ar-output.c
index 0b4b9a7..83b4616 100644
--- a/net/rxrpc/ar-output.c
+++ b/net/rxrpc/ar-output.c
@@ -45,7 +45,7 @@ static int rxrpc_sendmsg_cmsg(struct rxrpc_sock *rx, struct msghdr *msg,
if (msg->msg_controllen == 0)
return -EINVAL;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 634a2ab..a3802ed 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6592,8 +6592,7 @@ static int sctp_msghdr_parse(const struct msghdr *msg, sctp_cmsgs_t *cmsgs)
struct cmsghdr *cmsg;
struct msghdr *my_msg = (struct msghdr *)msg;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg != NULL;
- cmsg = CMSG_NXTHDR(my_msg, cmsg)) {
+ for_each_cmsghdr(cmsg, my_msg) {
if (!CMSG_OK(my_msg, cmsg))
return -EINVAL;
--
1.7.7
^ permalink raw reply related
* Re: PATCH] Revert "bridge: include in6.h in if_bridge.h for struct in6_addr"
From: David Miller @ 2014-12-10 4:49 UTC (permalink / raw)
To: stephen; +Cc: gregory.0xf0, f.fainelli, xiyou.wangcong, netdev
In-Reply-To: <20141209203007.188651df@urahara>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 9 Dec 2014 20:30:07 -0800
> This reverts commit 66f1c44887ba4f47d617f8ae21cf8e04e1892bd7.
>
> iproute2 uses sanitized kernel headers, and this change broke the build.
> The problem is that the iproute2 ip/iplink_bridge_slave.c include if_bridge.h
> but this causes a redefintion error when Glibc <netlink/in.h> is included.
>
> Since iproute2 is arguably the most important user of network headers,
> the uapi header change should be reverted.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Stephen, please understand first the reason for this change.
GLIBC and the kernel fight over various definitions of core
networking types.
The only way to resolve this in the long term is this change
in question.
If you can make iproute2 work with both the new and the old
mechanims, please do so instead of reverting this important
change.
^ permalink raw reply
* Re: [PATCH iproute2] ss: Use nl_proto_a2n for filtering by netlink proto
From: Stephen Hemminger @ 2014-12-10 4:40 UTC (permalink / raw)
To: Vadim Kochan; +Cc: netdev
In-Reply-To: <1417827139-10241-1-git-send-email-vadim4j@gmail.com>
On Sat, 6 Dec 2014 02:52:19 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:
> Now it is posible to filter by existing Netlink protos:
>
> ss -A netlink src uevent
> ss -A netlink src nft
> ss -A netlink src genl
>
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
Applied
^ permalink raw reply
* Re: [PATCH iproute2 0/2] lib names: Refactoring and cleanups
From: Stephen Hemminger @ 2014-12-10 4:38 UTC (permalink / raw)
To: Vadim Kochan; +Cc: netdev
In-Reply-To: <1417831512-19452-1-git-send-email-vadim4j@gmail.com>
On Sat, 6 Dec 2014 04:05:10 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:
> Some cleanups and refactoring in lib/rt_names.c:
>
> #1 Replaced using of /etc/iproute2 path by CONFDIR define
> when initializing tables of group names.
>
> #2 Added helper to have one func for parsing id and names from
> db files.
>
> Vadim Kochan (2):
> lib names: Use CONFDIR for specify 'group' file path
> lib names: Add helper func for parse id and name from file
>
> lib/rt_names.c | 70 +++++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 40 insertions(+), 30 deletions(-)
>
Both applied
^ permalink raw reply
* Re: [PATCH] iproute2: Add support for CAN presume-ack feature
From: Stephen Hemminger @ 2014-12-10 4:35 UTC (permalink / raw)
To: Nikita Edward Baruzdin; +Cc: linux-can, netdev
In-Reply-To: <1417768902-5404-2-git-send-email-nebaruzdin@gmail.com>
On Fri, 5 Dec 2014 11:41:42 +0300
Nikita Edward Baruzdin <nebaruzdin@gmail.com> wrote:
> This patch makes CAN_CTRLMODE_PRESUME_ACK netlink feature configurable.
> When enabled, the feature sets CAN controller in mode in which
> acknowledgement absence is ignored.
>
> Signed-off-by: Nikita Edward Baruzdin <nebaruzdin@gmail.com>
> ---
Applied.
^ permalink raw reply
* Re: [PATCH iproute2 REGRESSIONS v3] ss: Fix layout/output issues introduced by regression
From: Stephen Hemminger @ 2014-12-10 4:32 UTC (permalink / raw)
To: Vadim Kochan; +Cc: netdev
In-Reply-To: <1417799951-25143-1-git-send-email-vadim4j@gmail.com>
On Fri, 5 Dec 2014 19:19:11 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:
> This patch fixes the following issues which was introduced by me in commits:
>
> #1 (2dc854854b7f1b) ss: Fixed broken output for Netlink 'Peer Address:Port' column
> ISSUE: Broken layout when all sockets are printed out
>
> #2 (eef43b5052afb7) ss: Identify more netlink protocol names
> ISSUE: Protocol id is not printed if 'numbers only' output was specified (-n)
>
> Also aligned the width of the local/peer ports to be more wider.
>
> I tested with a lot of option combinations (I may miss some test cases),
> but layout seems to me better than the previous released version of iproute2/ss.
>
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
Applied.
Thanks for following through and dealing with this.
^ permalink raw reply
* PATCH] Revert "bridge: include in6.h in if_bridge.h for struct in6_addr"
From: Stephen Hemminger @ 2014-12-10 4:30 UTC (permalink / raw)
To: David Miller, Gregory Fong, Florian Fainelli, Cong Wang; +Cc: netdev
This reverts commit 66f1c44887ba4f47d617f8ae21cf8e04e1892bd7.
iproute2 uses sanitized kernel headers, and this change broke the build.
The problem is that the iproute2 ip/iplink_bridge_slave.c include if_bridge.h
but this causes a redefintion error when Glibc <netlink/in.h> is included.
Since iproute2 is arguably the most important user of network headers,
the uapi header change should be reverted.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
Please update stable as well.
include/uapi/linux/if_bridge.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index b03ee8f..439fd0d 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -15,7 +15,6 @@
#include <linux/types.h>
#include <linux/if_ether.h>
-#include <linux/in6.h>
#define SYSFS_BRIDGE_ATTR "bridge"
#define SYSFS_BRIDGE_FDB "brforward"
--
2.1.3
^ permalink raw reply related
* Re: [PATCH net-next] ipv6: fix sparse warning
From: David Miller @ 2014-12-10 4:18 UTC (permalink / raw)
To: ying.xue; +Cc: netdev
In-Reply-To: <1418182101-7992-1-git-send-email-ying.xue@windriver.com>
From: Ying Xue <ying.xue@windriver.com>
Date: Wed, 10 Dec 2014 11:28:21 +0800
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index 866d9c9..32bd4ad 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -524,11 +524,11 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
Again, changes to this file should not go through my tree.
^ permalink raw reply
* Re: [PATCH net-next] tipc: fix RCU sparse error
From: David Miller @ 2014-12-10 3:20 UTC (permalink / raw)
To: ying.xue; +Cc: jon.maloy, kbuild-all, erik.hugne, netdev, tipc-discussion
In-Reply-To: <1418180407-1847-1-git-send-email-ying.xue@windriver.com>
From: Ying Xue <ying.xue@windriver.com>
Date: Wed, 10 Dec 2014 11:00:07 +0800
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index aa79b3c..866d9c9 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -549,8 +549,8 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> */
> #define hlist_for_each_entry_from_rcu(pos, member) \
> for (; pos; \
> - pos = hlist_entry_safe(rcu_dereference((pos)->member.next),\
> - typeof(*(pos)), member))
> + pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu( \
> + &(pos)->member)), typeof(*(pos)), member))
>
> #endif /* __KERNEL__ */
> #endif
This needs to go via another route, perhaps the RCU maintainer.
^ permalink raw reply
* Re: [net-next PATCH 1/6] net: Split netdev_alloc_frag into __alloc_page_frag and add __napi_alloc_frag
From: Alexei Starovoitov @ 2014-12-10 4:16 UTC (permalink / raw)
To: Alexander Duyck
Cc: Network Development, David S. Miller, Eric Dumazet,
Jesper Dangaard Brouer
In-Reply-To: <20141210034042.2114.29360.stgit@ahduyck-vm-fedora20>
On Tue, Dec 9, 2014 at 7:40 PM, Alexander Duyck
<alexander.h.duyck@redhat.com> wrote:
> This patch splits the netdev_alloc_frag function up so that it can be used
> on one of two page frag pools instead of being fixed on the
> netdev_alloc_cache. By doing this we can add a NAPI specific function
> __napi_alloc_frag that accesses a pool that is only used from softirq
> context. The advantage to this is that we do not need to call
> local_irq_save/restore which can be a significant savings.
>
> I also took the opportunity to refactor the core bits that were placed in
> __alloc_page_frag. First I updated the allocation to do either a 32K
> allocation or an order 0 page. This is based on the changes in commmit
> d9b2938aa where it was found that latencies could be reduced in case of
thanks for explaining that piece of it.
> + struct page *page = NULL;
> + gfp_t gfp = gfp_mask;
> +
> + if (order) {
> + gfp_mask |= __GFP_COMP | __GFP_NOWARN | __GFP_NORETRY;
> + page = alloc_pages_node(NUMA_NO_NODE, gfp_mask, order);
> + nc->frag.size = PAGE_SIZE << (page ? order : 0);
> + }
>
> - local_irq_save(flags);
> - nc = this_cpu_ptr(&netdev_alloc_cache);
> - if (unlikely(!nc->frag.page)) {
> + if (unlikely(!page))
> + page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
I'm guessing you're not combining this 'if' with above one to
keep gfp untouched, so there is a 'warn' when it actually fails 2nd time.
Tricky :)
Anyway looks good to me and I think I understand it enough to say:
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
^ permalink raw reply
* Re: [PATCH v2 iproute2] ipaddress: enable -details option
From: Stephen Hemminger @ 2014-12-10 4:14 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: shemminger, netdev, mkubecek, jbenc
In-Reply-To: <1417711267-13334-1-git-send-email-nicolas.dichtel@6wind.com>
On Thu, 4 Dec 2014 17:41:07 +0100
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> This option was used only for 'ip link', but it can be useful to have it for
> 'ip address'. Thus it is possible to display link details and addresses with one
> command.
>
> Example:
> $ ip -d a ls dev gre1
> 9: gre1@NONE: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1468 qdisc noqueue state UNKNOWN group default
> link/gre 10.16.0.249 peer 10.16.0.121 promiscuity 0
> gre remote 10.16.0.121 local 10.16.0.249 ttl inherit ikey 0.0.0.10 okey 0.0.0.10 icsum ocsum
> inet 192.168.0.249 peer 192.168.0.121/32 scope global gre1
> valid_lft forever preferred_lft forever
> inet6 fe80::5efe:a10:f9/64 scope link
> valid_lft forever preferred_lft forever
>
> Suggested-by: Christophe Gouault <christophe.gouault@6wind.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Looks good, accepted.
^ permalink raw reply
* [net-next PATCH 6/6] ethernet/broadcom: Use napi_alloc_skb instead of netdev_alloc_skb_ip_align
From: Alexander Duyck @ 2014-12-10 3:41 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, eric.dumazet, Ariel Elior, brouer,
Gary Zambrano, davem, ast
In-Reply-To: <20141210033902.2114.68658.stgit@ahduyck-vm-fedora20>
This patch replaces the calls to netdev_alloc_skb_ip_align in the
copybreak paths.
Cc: Gary Zambrano <zambrano@broadcom.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ariel Elior <ariel.elior@qlogic.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/broadcom/b44.c | 2 +-
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 2 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index ffeaf47..d86d6ba 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -836,7 +836,7 @@ static int b44_rx(struct b44 *bp, int budget)
struct sk_buff *copy_skb;
b44_recycle_rx(bp, cons, bp->rx_prod);
- copy_skb = netdev_alloc_skb_ip_align(bp->dev, len);
+ copy_skb = napi_alloc_skb(&bp->napi, len);
if (copy_skb == NULL)
goto drop_it_no_recycle;
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 3e8d1a8..21206d3 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -385,7 +385,7 @@ static int bcm_enet_receive_queue(struct net_device *dev, int budget)
if (len < copybreak) {
struct sk_buff *nskb;
- nskb = netdev_alloc_skb_ip_align(dev, len);
+ nskb = napi_alloc_skb(&priv->napi, len);
if (!nskb) {
/* forget packet, just rearm desc */
dev->stats.rx_dropped++;
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index b4d71fd..1d1147c 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -1015,7 +1015,7 @@ static int bnx2x_rx_int(struct bnx2x_fastpath *fp, int budget)
*/
if ((bp->dev->mtu > ETH_MAX_PACKET_SIZE) &&
(len <= RX_COPY_THRESH)) {
- skb = netdev_alloc_skb_ip_align(bp->dev, len);
+ skb = napi_alloc_skb(&fp->napi, len);
if (skb == NULL) {
DP(NETIF_MSG_RX_ERR | NETIF_MSG_RX_STATUS,
"ERROR packet dropped because of alloc failure\n");
^ permalink raw reply related
* Re: [PATCH v8 3/3] net: hisilicon: new hip04 ethernet driver
From: Ding Tianhong @ 2014-12-10 3:51 UTC (permalink / raw)
To: Arnd Bergmann, Alexander Graf
Cc: Zhangfei Gao, davem, linux, f.fainelli, sergei.shtylyov,
mark.rutland, David.Laight, eric.dumazet, xuwei5,
linux-arm-kernel, netdev, devicetree
In-Reply-To: <2619239.NTtdNaZCJM@wuerfel>
On 2014/12/8 4:09, Arnd Bergmann wrote:
> On Sunday 07 December 2014 10:49:12 Alexander Graf wrote:
>> On 07.12.14 04:28, Ding Tianhong wrote:
>>> On 2014/12/7 8:42, Alexander Graf wrote:
>>>> On 19.04.14 03:13, Zhangfei Gao wrote:
>>>>> Support Hisilicon hip04 ethernet driver, including 100M / 1000M controller.
>>>>> The controller has no tx done interrupt, reclaim xmitted buffer in the poll.
>>>>>
>>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>>
>>>> Is this driver still supposed to go upstream? I presume this was the
>>>> last submission and it's been quite some time ago
>>>>
>>>
>>> yes, it is really a long time, but The hip04 did not support tx irq,
>>> we couldn't get any better idea to fix this defect, do you have any suggestion?
>>
>> Well, if hardware doesn't have a TX irq I don't see there's anything we
>> can do to fix that ;).
>
> I don't know if it's related to the ethernet on hip01, but I would assume
> it is, and that platform is currently being submitted for inclusion, so
> I'd definitely hope to see this driver get merged too eventually.
>
> IIRC, the last revision of the patch set had basically fixed the problem,
> except for a race that would still allow the napi poll function to exit
> with poll_complete() but a full queue of TX descriptors and no fallback
> to clean them up. There was also still an open question about whether or
> not the driver should use skb_orphan, but I may be misremembering that part.
>
Hi Arnd:
what about use a state machine to check the tx queue and free the skb, just like:
diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index 8593658..71faca8 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -396,9 +396,25 @@ static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
stats->tx_packets++;
priv->tx_count++;
+
+ queue_delayed_work(priv->wq, &priv->tx_queue, delay);
+
return NETDEV_TX_OK;
}
+static void hip04_tx_queue_monitor(struct work_struct *work)
+{
+ struct hip04_priv *priv = container_of(work, struct hip04_priv,
+ queue_work.work);
+ struct net_device *dev = priv->ndev;
+ hip04_tx_reclain(ndev, false);
+
+ if (TX_QUEUE_IS_EMPRY(ndev))
+ return;
+
+ queue_delayed_work(priv->wq, &priv->tx_queue, delay);
+}
+
static int hip04_rx_poll(struct napi_struct *napi, int budget)
{
struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
@@ -736,6 +752,8 @@ static int hip04_mac_probe(struct platform_device *pdev)
goto alloc_fail;
}
+ INIT_DELAYED_WORK(&priv->tx_queue, hip04_tx_queue_monitor);
+
return 0;
what do you think of this solution?
Regards
Ding
>> Dave, what's your take here? Should we keep a driver from going upstream
>> just because the hardware is partly broken? I'd really prefer to have an
>> upstream driver on that SoC rather than some random (eventually even
>> more broken) downstream code.
>
> We can certainly have a slow driver for this hardware, and I'd much
> prefer slow over broken. I'd guess that some of the performance impact
> of the missing interrupts can now be offset with the xmit_more logic.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> .
>
^ permalink raw reply related
* [net-next PATCH 5/6] ethernet/realtek: use napi_alloc_skb instead of netdev_alloc_skb_ip_align
From: Alexander Duyck @ 2014-12-10 3:41 UTC (permalink / raw)
To: netdev; +Cc: Realtek linux nic maintainers, ast, davem, eric.dumazet, brouer
In-Reply-To: <20141210033902.2114.68658.stgit@ahduyck-vm-fedora20>
This replaces most of the calls to netdev_alloc_skb_ip_align in the Realtek
drivers. The one instance I didn't replace in 8139cp.c is because it was
called as a part of init and as such is not always accessed from the
softirq context.
Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/realtek/8139cp.c | 2 +-
drivers/net/ethernet/realtek/8139too.c | 2 +-
drivers/net/ethernet/realtek/r8169.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 75b1693..9c31e46 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -507,7 +507,7 @@ rx_status_loop:
netif_dbg(cp, rx_status, dev, "rx slot %d status 0x%x len %d\n",
rx_tail, status, len);
- new_skb = netdev_alloc_skb_ip_align(dev, buflen);
+ new_skb = napi_alloc_skb(napi, buflen);
if (!new_skb) {
dev->stats.rx_dropped++;
goto rx_next;
diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 63dc0f9..6d0b9df 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -2037,7 +2037,7 @@ keep_pkt:
/* Malloc up new buffer, compatible with net-2e. */
/* Omit the four octet CRC from the length. */
- skb = netdev_alloc_skb_ip_align(dev, pkt_size);
+ skb = napi_alloc_skb(&tp->napi, pkt_size);
if (likely(skb)) {
#if RX_BUF_IDX == 3
wrap_copy(skb, rx_ring, ring_offset+4, pkt_size);
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 1620184..54a3ec1 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7269,7 +7269,7 @@ static struct sk_buff *rtl8169_try_rx_copy(void *data,
data = rtl8169_align(data);
dma_sync_single_for_cpu(d, addr, pkt_size, DMA_FROM_DEVICE);
prefetch(data);
- skb = netdev_alloc_skb_ip_align(tp->dev, pkt_size);
+ skb = napi_alloc_skb(&tp->napi, pkt_size);
if (skb)
memcpy(skb->data, data, pkt_size);
dma_sync_single_for_device(d, addr, pkt_size, DMA_FROM_DEVICE);
^ permalink raw reply related
* [net-next PATCH 4/6] cxgb: Use napi_alloc_skb instead of netdev_alloc_skb_ip_align
From: Alexander Duyck @ 2014-12-10 3:41 UTC (permalink / raw)
To: netdev; +Cc: ast, davem, eric.dumazet, brouer
In-Reply-To: <20141210033902.2114.68658.stgit@ahduyck-vm-fedora20>
In order to use napi_alloc_skb I needed to pass a pointer to struct adapter
instead of struct pci_dev. This allowed me to access &adapter->napi.
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/chelsio/cxgb/sge.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.c b/drivers/net/ethernet/chelsio/cxgb/sge.c
index 86222a1..babe2a9 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.c
@@ -1025,7 +1025,7 @@ MODULE_PARM_DESC(copybreak, "Receive copy threshold");
/**
* get_packet - return the next ingress packet buffer
- * @pdev: the PCI device that received the packet
+ * @adapter: the adapter that received the packet
* @fl: the SGE free list holding the packet
* @len: the actual packet length, excluding any SGE padding
*
@@ -1037,14 +1037,15 @@ MODULE_PARM_DESC(copybreak, "Receive copy threshold");
* threshold and the packet is too big to copy, or (b) the packet should
* be copied but there is no memory for the copy.
*/
-static inline struct sk_buff *get_packet(struct pci_dev *pdev,
+static inline struct sk_buff *get_packet(struct adapter *adapter,
struct freelQ *fl, unsigned int len)
{
- struct sk_buff *skb;
const struct freelQ_ce *ce = &fl->centries[fl->cidx];
+ struct pci_dev *pdev = adapter->pdev;
+ struct sk_buff *skb;
if (len < copybreak) {
- skb = netdev_alloc_skb_ip_align(NULL, len);
+ skb = napi_alloc_skb(&adapter->napi, len);
if (!skb)
goto use_orig_buf;
@@ -1357,7 +1358,7 @@ static void sge_rx(struct sge *sge, struct freelQ *fl, unsigned int len)
struct sge_port_stats *st;
struct net_device *dev;
- skb = get_packet(adapter->pdev, fl, len - sge->rx_pkt_pad);
+ skb = get_packet(adapter, fl, len - sge->rx_pkt_pad);
if (unlikely(!skb)) {
sge->stats.rx_drops++;
return;
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox