* Re: [PATCH net] core: Untag packets after rx_handler has run.
From: Jiri Pirko @ 2014-09-04 19:05 UTC (permalink / raw)
To: Vladislav Yasevich
Cc: netdev, Vladislav Yasevich, Florian Zumbiehl, Eric Dumazet,
Matthew Rosato
In-Reply-To: <1409856043-21840-1-git-send-email-vyasevic@redhat.com>
Thu, Sep 04, 2014 at 08:40:43PM CEST, vyasevich@gmail.com wrote:
>Currently, we attempt to remove the vlan informaion from the packet
>before passing it to the rx_handler. In most situations this works
>just fine since the rx_handlers are usually installed for the
>lower device and thus vlan device isn't found. However, macvtap
>device is a bit different as it installs an rx_handler on top
>of a macvlan device. As a result, if someone was define a vlan
>device on top of a macvap (for the purposes of enabling a VM
>to use vlans with macvtap), then the current code will result
>in passing an untagged packet to the macvtap rx_handler and the
>VM will not receive tagged traffic.
skb->vlan_tci is set. macvlan should work with that to pass the frame
correctly. This should be handled in macvtap code.
btw can you give me an example of setup where rx_handler is set for
macvlan device? I wonder what is could be.
>
>This patch moves the untaggin code to after the rx_handler for
>the current device has been called. This still works for the
>existing rx_handlers (like bonding/teaming/bridging/etc) and
>also makes vlans on top of macvtap work as before.
>
>Fixes: 6acf54f1cf (macvtap: Add support of packet capture on macvtap device)
>Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>---
> net/core/dev.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index ab9a165..54691d1 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -3642,17 +3642,6 @@ ncls:
> if (pfmemalloc && !skb_pfmemalloc_protocol(skb))
> goto drop;
>
>- if (vlan_tx_tag_present(skb)) {
>- if (pt_prev) {
>- ret = deliver_skb(skb, pt_prev, orig_dev);
>- pt_prev = NULL;
>- }
>- if (vlan_do_receive(&skb))
>- goto another_round;
>- else if (unlikely(!skb))
>- goto unlock;
>- }
>-
> rx_handler = rcu_dereference(skb->dev->rx_handler);
> if (rx_handler) {
> if (pt_prev) {
>@@ -3674,6 +3663,17 @@ ncls:
> }
> }
>
>+ if (vlan_tx_tag_present(skb)) {
>+ if (pt_prev) {
>+ ret = deliver_skb(skb, pt_prev, orig_dev);
>+ pt_prev = NULL;
>+ }
>+ if (vlan_do_receive(&skb))
>+ goto another_round;
>+ else if (unlikely(!skb))
>+ goto unlock;
>+ }
>+
nack. This will definitelly break several stacked setups.
> if (unlikely(vlan_tx_tag_present(skb))) {
> if (vlan_tx_tag_get_id(skb))
> skb->pkt_type = PACKET_OTHERHOST;
>--
>1.9.3
>
^ permalink raw reply
* [PATCH] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Vadim Kochan @ 2014-09-04 18:52 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
This is ugly fix but solves the case when timestamp
or banner-label is printed before the cloned route will be skipped
by iproute filter which filters out all cached routes by default.
In such case timestamp will be printed twice:
Timestamp: Thu Sep 4 19:46:59 2014 457933 usec
Timestamp: Thu Sep 4 19:47:07 2014 977970 usec
10.3.5.1 dev wlp3s0 lladdr XX:XX:XX:XX:XX:XX STALE
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
ip/ipmonitor.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 70f2a7a..2bd3fed 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -41,9 +41,6 @@ static int accept_msg(const struct sockaddr_nl *who,
{
FILE *fp = (FILE*)arg;
- if (timestamp)
- print_timestamp(fp);
-
if (n->nlmsg_type == RTM_NEWROUTE || n->nlmsg_type == RTM_DELROUTE) {
struct rtmsg *r = NLMSG_DATA(n);
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
@@ -53,6 +50,12 @@ static int accept_msg(const struct sockaddr_nl *who,
return -1;
}
+ if (r->rtm_flags&RTM_F_CLONED)
+ return 0;
+
+ if (timestamp)
+ print_timestamp(fp);
+
if (r->rtm_family == RTNL_FAMILY_IPMR ||
r->rtm_family == RTNL_FAMILY_IP6MR) {
if (prefix_banner)
@@ -66,6 +69,10 @@ static int accept_msg(const struct sockaddr_nl *who,
return 0;
}
}
+
+ if (timestamp)
+ print_timestamp(fp);
+
if (n->nlmsg_type == RTM_NEWLINK || n->nlmsg_type == RTM_DELLINK) {
ll_remember_index(who, n, NULL);
if (prefix_banner)
--
2.1.0
^ permalink raw reply related
* [PATCH net] core: Untag packets after rx_handler has run.
From: Vladislav Yasevich @ 2014-09-04 18:40 UTC (permalink / raw)
To: netdev
Cc: Vladislav Yasevich, Jiri Pirko, Florian Zumbiehl, Eric Dumazet,
Matthew Rosato
Currently, we attempt to remove the vlan informaion from the packet
before passing it to the rx_handler. In most situations this works
just fine since the rx_handlers are usually installed for the
lower device and thus vlan device isn't found. However, macvtap
device is a bit different as it installs an rx_handler on top
of a macvlan device. As a result, if someone was define a vlan
device on top of a macvap (for the purposes of enabling a VM
to use vlans with macvtap), then the current code will result
in passing an untagged packet to the macvtap rx_handler and the
VM will not receive tagged traffic.
This patch moves the untaggin code to after the rx_handler for
the current device has been called. This still works for the
existing rx_handlers (like bonding/teaming/bridging/etc) and
also makes vlans on top of macvtap work as before.
Fixes: 6acf54f1cf (macvtap: Add support of packet capture on macvtap device)
Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/core/dev.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index ab9a165..54691d1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3642,17 +3642,6 @@ ncls:
if (pfmemalloc && !skb_pfmemalloc_protocol(skb))
goto drop;
- if (vlan_tx_tag_present(skb)) {
- if (pt_prev) {
- ret = deliver_skb(skb, pt_prev, orig_dev);
- pt_prev = NULL;
- }
- if (vlan_do_receive(&skb))
- goto another_round;
- else if (unlikely(!skb))
- goto unlock;
- }
-
rx_handler = rcu_dereference(skb->dev->rx_handler);
if (rx_handler) {
if (pt_prev) {
@@ -3674,6 +3663,17 @@ ncls:
}
}
+ if (vlan_tx_tag_present(skb)) {
+ if (pt_prev) {
+ ret = deliver_skb(skb, pt_prev, orig_dev);
+ pt_prev = NULL;
+ }
+ if (vlan_do_receive(&skb))
+ goto another_round;
+ else if (unlikely(!skb))
+ goto unlock;
+ }
+
if (unlikely(vlan_tx_tag_present(skb))) {
if (vlan_tx_tag_get_id(skb))
skb->pkt_type = PACKET_OTHERHOST;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next v4 2/3] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Rick Jones @ 2014-09-04 18:33 UTC (permalink / raw)
To: Alexander Duyck, netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <5408AFC7.70709@intel.com>
>> I may not be following the flow correctly, and may be noticing only
>> because I just did two "floor-sweeping" patches to shift be2net and
>> mlx4_en to "consume" but would it be better if these kfree_skb calls
>> were a "consume" variety?
>>
>> rick jones
>
> kfree_skb is probably the correct approach. In this case it represents
> a buffer that has to be freed due to a Tx timestamp request timeout so
> it would be an event that we would want to trace as an error event.
Thanks.
rick
^ permalink raw reply
* Re: [PATCH net-next v4 2/3] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Alexander Duyck @ 2014-09-04 18:30 UTC (permalink / raw)
To: Rick Jones, netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <5408A60A.3070603@hp.com>
On 09/04/2014 10:48 AM, Rick Jones wrote:
> On 09/04/2014 10:31 AM, Alexander Duyck wrote:
>
>>
>> ---
>>
>> v2: Renamed function to skb_clone_sk.
>> Added destructor to call sock_put instead of doing it ourselves.
>> Dropped freeing functionality from skb_complete_tx_timestamp.
>> Added additional documentation to the code.
>>
>> v3: Renamed destructor sock_efree and moved to sock.c/h
>> Added sock_hold/sock_put around call to sock_queue_err_skb
>>
>> v4: Dropped combining sock_edemux with sock_efree where the 2 are
>> identical
>>
>> drivers/net/phy/dp83640.c | 6 +++---
>> include/linux/skbuff.h | 2 ++
>> include/net/sock.h | 1 +
>> net/core/skbuff.c | 32 +++++++++++++++++++++++++-------
>> net/core/sock.c | 6 ++++++
>> net/core/timestamping.c | 14 +++-----------
>> 6 files changed, 40 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
>> index d5991ac..87648b3 100644
>> --- a/drivers/net/phy/dp83640.c
>> +++ b/drivers/net/phy/dp83640.c
>> @@ -1148,7 +1148,7 @@ static void dp83640_remove(struct phy_device
>> *phydev)
>> kfree_skb(skb);
>>
>> while ((skb = skb_dequeue(&dp83640->tx_queue)) != NULL)
>> - skb_complete_tx_timestamp(skb, NULL);
>> + kfree_skb(skb);
>
> I may not be following the flow correctly, and may be noticing only
> because I just did two "floor-sweeping" patches to shift be2net and
> mlx4_en to "consume" but would it be better if these kfree_skb calls
> were a "consume" variety?
>
> rick jones
kfree_skb is probably the correct approach. In this case it represents
a buffer that has to be freed due to a Tx timestamp request timeout so
it would be an event that we would want to trace as an error event.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH net-next] sock: consistent errqueue errors and signals
From: Willem de Bruijn @ 2014-09-04 18:18 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Network Development, David Miller
In-Reply-To: <CA+FuTSeJDbWN82LOZ5MoHtdY6Qe1GywBMC2QGrT3Z-n7O0727w@mail.gmail.com>
On Thu, Sep 4, 2014 at 2:13 PM, Willem de Bruijn <willemb@google.com> wrote:
>> I was concerned that we report the error twice. Once in the sendmsg path
>> and again on recvmsg or sendmsg, which finally clears sk_err.
>
> I had overlooked that messages can be queued onto the error queue
> during send call processing (most cases of ip_local_error) as well as from
> asynchronous events (ip_icmp_error, skb_complete_tx_timestamp, ..).
>
> For the second case, the patch fixes that an enqueued message does
> not have its error returned on the first subsequent system call. By
> setting sk->sk_err unconditionally, the patch indeed introduces a
> double notification in the other case.
>
> Both paths can be made correct if sk->sk_err is set on enqueue
> only when not called from inside the send() context. The easiest
> change for timestamping is to set it in __skb_tstamp_tx directly.
> Slightly more general would be to add
>
> * /* queue errors from asynchronous events: report the error on the
> next syscall */
> + int sock_queue_err_skb_and_notify(struct sock *sk, struct sk_buff *skb)
> + {
> + int ret = sock_queue_err_skb(sk);
> + if (!ret)
> + sk->sk_err = SKB_EXT_ERR(skb)->ee.ee_errno;
> + }
This was just a quick, buggy, sketch. The errno should be read and
cached locally before calling sock_queue_err_skb.
+ int ret, errno;
+
+ errno = SKB_EXT_ERR(skb)->ee.ee_errno;
+ ret = sock_queue_err_skb(sk, skb);
+ if (!ret)
+ sk->sk_err = errno;
The assignment is also not atomic and not protected by the socket
lock here. Neither is it on dequeue, but this warrants looking into.
> and call this from __skb_tstamp_tx, skb_complete_tx_timestamp,
> skb_complete_wifi_ack. It is perhaps also safe to call from
> ip[v6]_icmp_error, but as you point out there are many legacy expectations
> there, so it may be best to leave that code path as is.
>
>>> If socket option IP_RECVERR is set, the process should immediately
>>> read the error queue when a send call fails with EMSGSIZE or any other
>>> relevant error. In that case, the error is not reported more than
>>> once.
>>
>> I agree but am afraid of applications start to break.
>>
>>> The alternative patch does not overwrite sk->sk_err, but results in
>>> this same modified user-visible behavior.
>>>
>>> Without the patch, after a send call fails in the above code, the
>>> process can continue calling send/recv normally while ignoring the
>>> error, since all "if (sk->sk_err)" checks fall through. If we have to
>>> treat this as legacy behavior, then neither patch should be applied.
>>> Then, the semantics are that queued errors are non-blocking. In that
>>> case, the only patch needed for consistent semantics is to remove the
>>> sk->sk_err assigment in skb_dequeue_err_skb.
>>
>> I am afraid that we need to go down this route. :/
>>
>> I would even let the sk->sk_error_report call be handled by the protocol
>> error reporting function, as it would need to deal with sk->sk_err, too.
>>
>> Most applications susceptible to breakage here are ping and
>> tracepath/traceroute. I don't think they all check MSG_ERRQUEUE always
>> on every error. But it shouldn't matter that much because they are only
>> short running apps and error queue will be freed if application exits
>> before socket accounting will shut down the socket. If we start to
>> report one instance of an error event multiple times, I think they might
>> become confused.
>>
>>> On a related note, when returning an error, returning the errno from
>>> the item on the queue is a confusing signal. Some error codes are the
>>> result of protocol processing and have nothing queued (e.g., EINVAL),
>>> others are due to an queued error (ENOMSG) and some are simply
>>> ambiguous (EMSGSIZE). It is not reasonable to expect processes to
>>> figure out the three sets. One solution would be to dedicate a special
>>> error code to denote "there is a message queued onto sk_error_queue,
>>> read the actual errno from there". This choice only relevant if we
>>> decide to return an error, at all, of course.
>>
>> errno messages are higly sensitive regarding backwards compatibility. I
>> am totally with you that a special errno marker to indicate that a
>> packet was enqueued on the error queue would be much better. Currently
>> applications always have to query error queue on error indication,
>> otherwise socket accounting might shut down the socket at some point.
>> Sadly I don't see this change to happen.
>>
>> Bye,
>> Hannes
^ permalink raw reply
* Re: Does NetworkManager kill IPv6 connections?
From: Dan Williams @ 2014-09-04 18:17 UTC (permalink / raw)
To: David Laight; +Cc: netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174876CB@AcuExch.aculab.com>
On Thu, 2014-09-04 at 14:47 +0000, David Laight wrote:
> I was running a overnight test that failed because an (almost idle) IPv6 connection
> unexpectedly disconnected.
> 30 seconds before the disconnect syslog contains (retyped):
> NetworkManager[pid]: <info> policy set 'Wired connection 2' (eth0) as default for IPv6 routing and DNS.
> The same message appeared exactly 70 minutes earlier - with no ill effect.
> A second connection between the same pair if addresses (carrying a lot of traffic)
> was unaffected.
>
> Any idea what/why NetworkManager is doing?
> (Yes I know it is userspace, but it seems to be breaking the kernel....)
>
> The system is an ubuntu 13.04 desktop (3.8.0-19 kernel), that recent, but
> newer than the ones a lot of our customers use.
Is the version of the Ubuntu "network-manager" package on your systems
0.9.8.0-0ubuntu6.1?
If so, I know we've fixed a ton of issues with IPv6 since 0.9.8.0 (which
was released about 18 months ago), and the ones that may be affecting
you are related to router advertisement timing and expiry. The latest
stable NM version upstream is 0.9.10 and should contain all of those
fixes.
Since I'm pretty sure this isn't a kernel problem, would you mind
discussing this over on the NM lists here:
https://mail.gnome.org/mailman/listinfo/networkmanager-list
If you don't want to subscribe that's fine, I'll moderate your message
through and we can just use "reply all".
Thanks,
Dan
> I can rerun the test, but don't know what to look for.
> 'eth0' is the only ethernet interface in the system that is actually
> connected to a network.
>
> Some indications of where to look would be appreciated.
>
> David
>
>
>
> --
> 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: [PATCH net-next] sock: consistent errqueue errors and signals
From: Willem de Bruijn @ 2014-09-04 18:13 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Network Development, David Miller
In-Reply-To: <1409789836.3347097.163379629.7D1B7A44@webmail.messagingengine.com>
> I was concerned that we report the error twice. Once in the sendmsg path
> and again on recvmsg or sendmsg, which finally clears sk_err.
I had overlooked that messages can be queued onto the error queue
during send call processing (most cases of ip_local_error) as well as from
asynchronous events (ip_icmp_error, skb_complete_tx_timestamp, ..).
For the second case, the patch fixes that an enqueued message does
not have its error returned on the first subsequent system call. By
setting sk->sk_err unconditionally, the patch indeed introduces a
double notification in the other case.
Both paths can be made correct if sk->sk_err is set on enqueue
only when not called from inside the send() context. The easiest
change for timestamping is to set it in __skb_tstamp_tx directly.
Slightly more general would be to add
* /* queue errors from asynchronous events: report the error on the
next syscall */
+ int sock_queue_err_skb_and_notify(struct sock *sk, struct sk_buff *skb)
+ {
+ int ret = sock_queue_err_skb(sk);
+ if (!ret)
+ sk->sk_err = SKB_EXT_ERR(skb)->ee.ee_errno;
+ }
and call this from __skb_tstamp_tx, skb_complete_tx_timestamp,
skb_complete_wifi_ack. It is perhaps also safe to call from
ip[v6]_icmp_error, but as you point out there are many legacy expectations
there, so it may be best to leave that code path as is.
>> If socket option IP_RECVERR is set, the process should immediately
>> read the error queue when a send call fails with EMSGSIZE or any other
>> relevant error. In that case, the error is not reported more than
>> once.
>
> I agree but am afraid of applications start to break.
>
>> The alternative patch does not overwrite sk->sk_err, but results in
>> this same modified user-visible behavior.
>>
>> Without the patch, after a send call fails in the above code, the
>> process can continue calling send/recv normally while ignoring the
>> error, since all "if (sk->sk_err)" checks fall through. If we have to
>> treat this as legacy behavior, then neither patch should be applied.
>> Then, the semantics are that queued errors are non-blocking. In that
>> case, the only patch needed for consistent semantics is to remove the
>> sk->sk_err assigment in skb_dequeue_err_skb.
>
> I am afraid that we need to go down this route. :/
>
> I would even let the sk->sk_error_report call be handled by the protocol
> error reporting function, as it would need to deal with sk->sk_err, too.
>
> Most applications susceptible to breakage here are ping and
> tracepath/traceroute. I don't think they all check MSG_ERRQUEUE always
> on every error. But it shouldn't matter that much because they are only
> short running apps and error queue will be freed if application exits
> before socket accounting will shut down the socket. If we start to
> report one instance of an error event multiple times, I think they might
> become confused.
>
>> On a related note, when returning an error, returning the errno from
>> the item on the queue is a confusing signal. Some error codes are the
>> result of protocol processing and have nothing queued (e.g., EINVAL),
>> others are due to an queued error (ENOMSG) and some are simply
>> ambiguous (EMSGSIZE). It is not reasonable to expect processes to
>> figure out the three sets. One solution would be to dedicate a special
>> error code to denote "there is a message queued onto sk_error_queue,
>> read the actual errno from there". This choice only relevant if we
>> decide to return an error, at all, of course.
>
> errno messages are higly sensitive regarding backwards compatibility. I
> am totally with you that a special errno marker to indicate that a
> packet was enqueued on the error queue would be much better. Currently
> applications always have to query error queue on error indication,
> otherwise socket accounting might shut down the socket at some point.
> Sadly I don't see this change to happen.
>
> Bye,
> Hannes
^ permalink raw reply
* Re: [PATCH net-next v4 2/3] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Rick Jones @ 2014-09-04 17:48 UTC (permalink / raw)
To: Alexander Duyck, netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140904173116.7702.30877.stgit@ahduyck-bv4.jf.intel.com>
On 09/04/2014 10:31 AM, Alexander Duyck wrote:
>
> ---
>
> v2: Renamed function to skb_clone_sk.
> Added destructor to call sock_put instead of doing it ourselves.
> Dropped freeing functionality from skb_complete_tx_timestamp.
> Added additional documentation to the code.
>
> v3: Renamed destructor sock_efree and moved to sock.c/h
> Added sock_hold/sock_put around call to sock_queue_err_skb
>
> v4: Dropped combining sock_edemux with sock_efree where the 2 are identical
>
> drivers/net/phy/dp83640.c | 6 +++---
> include/linux/skbuff.h | 2 ++
> include/net/sock.h | 1 +
> net/core/skbuff.c | 32 +++++++++++++++++++++++++-------
> net/core/sock.c | 6 ++++++
> net/core/timestamping.c | 14 +++-----------
> 6 files changed, 40 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
> index d5991ac..87648b3 100644
> --- a/drivers/net/phy/dp83640.c
> +++ b/drivers/net/phy/dp83640.c
> @@ -1148,7 +1148,7 @@ static void dp83640_remove(struct phy_device *phydev)
> kfree_skb(skb);
>
> while ((skb = skb_dequeue(&dp83640->tx_queue)) != NULL)
> - skb_complete_tx_timestamp(skb, NULL);
> + kfree_skb(skb);
I may not be following the flow correctly, and may be noticing only
because I just did two "floor-sweeping" patches to shift be2net and
mlx4_en to "consume" but would it be better if these kfree_skb calls
were a "consume" variety?
rick jones
>
> clock = dp83640_clock_get(dp83640->clock);
>
> @@ -1405,7 +1405,7 @@ static void dp83640_txtstamp(struct phy_device *phydev,
>
> case HWTSTAMP_TX_ONESTEP_SYNC:
> if (is_sync(skb, type)) {
> - skb_complete_tx_timestamp(skb, NULL);
> + kfree_skb(skb);
> return;
> }
> /* fall through */
> @@ -1416,7 +1416,7 @@ static void dp83640_txtstamp(struct phy_device *phydev,
>
> case HWTSTAMP_TX_OFF:
> default:
> - skb_complete_tx_timestamp(skb, NULL);
> + kfree_skb(skb);
> break;
> }
> }
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 02529fc..1cf0cfa 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2690,6 +2690,8 @@ static inline ktime_t net_invalid_timestamp(void)
> return ktime_set(0, 0);
> }
>
> +struct sk_buff *skb_clone_sk(struct sk_buff *skb);
> +
> #ifdef CONFIG_NETWORK_PHY_TIMESTAMPING
>
> void skb_clone_tx_timestamp(struct sk_buff *skb);
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 3fde613..e02be37 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1574,6 +1574,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force,
> void sock_wfree(struct sk_buff *skb);
> void skb_orphan_partial(struct sk_buff *skb);
> void sock_rfree(struct sk_buff *skb);
> +void sock_efree(struct sk_buff *skb);
> void sock_edemux(struct sk_buff *skb);
>
> int sock_setsockopt(struct socket *sock, int level, int op,
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 697e696..a936a40 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3511,6 +3511,27 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
> }
> EXPORT_SYMBOL(sock_dequeue_err_skb);
>
> +struct sk_buff *skb_clone_sk(struct sk_buff *skb)
> +{
> + struct sock *sk = skb->sk;
> + struct sk_buff *clone;
> +
> + if (!sk || !atomic_inc_not_zero(&sk->sk_refcnt))
> + return NULL;
> +
> + clone = skb_clone(skb, GFP_ATOMIC);
> + if (!clone) {
> + sock_put(sk);
> + return NULL;
> + }
> +
> + clone->sk = sk;
> + clone->destructor = sock_efree;
> +
> + return clone;
> +}
> +EXPORT_SYMBOL(skb_clone_sk);
> +
> static void __skb_complete_tx_timestamp(struct sk_buff *skb,
> struct sock *sk,
> int tstype)
> @@ -3540,14 +3561,11 @@ void skb_complete_tx_timestamp(struct sk_buff *skb,
> {
> struct sock *sk = skb->sk;
>
> - skb->sk = NULL;
> + /* take a reference to prevent skb_orphan() from freeing the socket */
> + sock_hold(sk);
>
> - if (hwtstamps) {
> - *skb_hwtstamps(skb) = *hwtstamps;
> - __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
> - } else {
> - kfree_skb(skb);
> - }
> + *skb_hwtstamps(skb) = *hwtstamps;
> + __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
>
> sock_put(sk);
> }
> diff --git a/net/core/sock.c b/net/core/sock.c
> index f1a638e..d04005c 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1637,6 +1637,12 @@ void sock_rfree(struct sk_buff *skb)
> }
> EXPORT_SYMBOL(sock_rfree);
>
> +void sock_efree(struct sk_buff *skb)
> +{
> + sock_put(skb->sk);
> +}
> +EXPORT_SYMBOL(sock_efree);
> +
> void sock_edemux(struct sk_buff *skb)
> {
> struct sock *sk = skb->sk;
> diff --git a/net/core/timestamping.c b/net/core/timestamping.c
> index f48a59f..43d3dd6 100644
> --- a/net/core/timestamping.c
> +++ b/net/core/timestamping.c
> @@ -36,10 +36,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
> {
> struct phy_device *phydev;
> struct sk_buff *clone;
> - struct sock *sk = skb->sk;
> unsigned int type;
>
> - if (!sk)
> + if (!skb->sk)
> return;
>
> type = classify(skb);
> @@ -48,16 +47,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
>
> phydev = skb->dev->phydev;
> if (likely(phydev->drv->txtstamp)) {
> - if (!atomic_inc_not_zero(&sk->sk_refcnt))
> + clone = skb_clone_sk(skb);
> + if (!clone)
> return;
> -
> - clone = skb_clone(skb, GFP_ATOMIC);
> - if (!clone) {
> - sock_put(sk);
> - return;
> - }
> -
> - clone->sk = sk;
> phydev->drv->txtstamp(phydev, clone, type);
> }
> }
>
> --
> 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: Does NetworkManager kill IPv6 connections?
From: Daniel Borkmann @ 2014-09-04 17:43 UTC (permalink / raw)
To: David Laight; +Cc: netdev@vger.kernel.org, dcbw
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174876CB@AcuExch.aculab.com>
[ Cc'ing Dan ]
On 09/04/2014 04:47 PM, David Laight wrote:
> I was running a overnight test that failed because an (almost idle) IPv6 connection
> unexpectedly disconnected.
> 30 seconds before the disconnect syslog contains (retyped):
> NetworkManager[pid]: <info> policy set 'Wired connection 2' (eth0) as default for IPv6 routing and DNS.
> The same message appeared exactly 70 minutes earlier - with no ill effect.
> A second connection between the same pair if addresses (carrying a lot of traffic)
> was unaffected.
>
> Any idea what/why NetworkManager is doing?
> (Yes I know it is userspace, but it seems to be breaking the kernel....)
>
> The system is an ubuntu 13.04 desktop (3.8.0-19 kernel), that recent, but
> newer than the ones a lot of our customers use.
>
> I can rerun the test, but don't know what to look for.
> 'eth0' is the only ethernet interface in the system that is actually
> connected to a network.
>
> Some indications of where to look would be appreciated.
>
> David
^ permalink raw reply
* [PATCH net-next v4 3/3] net: merge cases where sock_efree and sock_edemux are the same function
From: Alexander Duyck @ 2014-09-04 17:32 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140904172906.7702.87598.stgit@ahduyck-bv4.jf.intel.com>
Since sock_efree and sock_demux are essentially the same code for non-TCP
sockets and the case where CONFIG_INET is not defined we can combine the
code or replace the call to sock_edemux in several spots. As a result we
can avoid a bit of unnecessary code or code duplication.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
v4: Added this change as a separate patch.
include/net/sock.h | 4 ++++
net/core/sock.c | 4 ++--
net/ipv4/udp.c | 2 +-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index e02be37..ad23e80 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1575,7 +1575,11 @@ void sock_wfree(struct sk_buff *skb);
void skb_orphan_partial(struct sk_buff *skb);
void sock_rfree(struct sk_buff *skb);
void sock_efree(struct sk_buff *skb);
+#ifdef CONFIG_INET
void sock_edemux(struct sk_buff *skb);
+#else
+#define sock_edemux(skb) sock_efree(skb)
+#endif
int sock_setsockopt(struct socket *sock, int level, int op,
char __user *optval, unsigned int optlen);
diff --git a/net/core/sock.c b/net/core/sock.c
index d04005c..69592cb 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1643,18 +1643,18 @@ void sock_efree(struct sk_buff *skb)
}
EXPORT_SYMBOL(sock_efree);
+#ifdef CONFIG_INET
void sock_edemux(struct sk_buff *skb)
{
struct sock *sk = skb->sk;
-#ifdef CONFIG_INET
if (sk->sk_state == TCP_TIME_WAIT)
inet_twsk_put(inet_twsk(sk));
else
-#endif
sock_put(sk);
}
EXPORT_SYMBOL(sock_edemux);
+#endif
kuid_t sock_i_uid(struct sock *sk)
{
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0da3849..cd0db54 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1972,7 +1972,7 @@ void udp_v4_early_demux(struct sk_buff *skb)
return;
skb->sk = sk;
- skb->destructor = sock_edemux;
+ skb->destructor = sock_efree;
dst = sk->sk_rx_dst;
if (dst)
^ permalink raw reply related
* [PATCH net-next v4 1/3] net-timestamp: Merge shared code between phy and regular timestamping
From: Alexander Duyck @ 2014-09-04 17:31 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140904172906.7702.87598.stgit@ahduyck-bv4.jf.intel.com>
This change merges the shared bits that exist between skb_tx_tstamp and
skb_complete_tx_timestamp. By doing this we can avoid the two diverging as
there were already changes pushed into skb_tx_tstamp that hadn't made it
into the other function.
In addition this resolves issues with the fact that
skb_complete_tx_timestamp was included in linux/skbuff.h even though it was
only compiled in if phy timestamping was enabled.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
v2: Dropped comment and restored original order based on comments
from Willem de Bruijn.
v3: No changes.
v4: No changes.
net/core/skbuff.c | 65 ++++++++++++++++++++++++++++++-----------------
net/core/timestamping.c | 29 ---------------------
2 files changed, 42 insertions(+), 52 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 53ce536..697e696 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3511,33 +3511,13 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
}
EXPORT_SYMBOL(sock_dequeue_err_skb);
-void __skb_tstamp_tx(struct sk_buff *orig_skb,
- struct skb_shared_hwtstamps *hwtstamps,
- struct sock *sk, int tstype)
+static void __skb_complete_tx_timestamp(struct sk_buff *skb,
+ struct sock *sk,
+ int tstype)
{
struct sock_exterr_skb *serr;
- struct sk_buff *skb;
int err;
- if (!sk)
- return;
-
- if (hwtstamps) {
- *skb_hwtstamps(orig_skb) =
- *hwtstamps;
- } else {
- /*
- * no hardware time stamps available,
- * so keep the shared tx_flags and only
- * store software time stamp
- */
- orig_skb->tstamp = ktime_get_real();
- }
-
- skb = skb_clone(orig_skb, GFP_ATOMIC);
- if (!skb)
- return;
-
serr = SKB_EXT_ERR(skb);
memset(serr, 0, sizeof(*serr));
serr->ee.ee_errno = ENOMSG;
@@ -3554,6 +3534,45 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
if (err)
kfree_skb(skb);
}
+
+void skb_complete_tx_timestamp(struct sk_buff *skb,
+ struct skb_shared_hwtstamps *hwtstamps)
+{
+ struct sock *sk = skb->sk;
+
+ skb->sk = NULL;
+
+ if (hwtstamps) {
+ *skb_hwtstamps(skb) = *hwtstamps;
+ __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
+ } else {
+ kfree_skb(skb);
+ }
+
+ sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(skb_complete_tx_timestamp);
+
+void __skb_tstamp_tx(struct sk_buff *orig_skb,
+ struct skb_shared_hwtstamps *hwtstamps,
+ struct sock *sk, int tstype)
+{
+ struct sk_buff *skb;
+
+ if (!sk)
+ return;
+
+ if (hwtstamps)
+ *skb_hwtstamps(orig_skb) = *hwtstamps;
+ else
+ orig_skb->tstamp = ktime_get_real();
+
+ skb = skb_clone(orig_skb, GFP_ATOMIC);
+ if (!skb)
+ return;
+
+ __skb_complete_tx_timestamp(skb, sk, tstype);
+}
EXPORT_SYMBOL_GPL(__skb_tstamp_tx);
void skb_tstamp_tx(struct sk_buff *orig_skb,
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index a877039..f48a59f 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -63,35 +63,6 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(skb_clone_tx_timestamp);
-void skb_complete_tx_timestamp(struct sk_buff *skb,
- struct skb_shared_hwtstamps *hwtstamps)
-{
- struct sock *sk = skb->sk;
- struct sock_exterr_skb *serr;
- int err;
-
- if (!hwtstamps) {
- sock_put(sk);
- kfree_skb(skb);
- return;
- }
-
- *skb_hwtstamps(skb) = *hwtstamps;
-
- serr = SKB_EXT_ERR(skb);
- memset(serr, 0, sizeof(*serr));
- serr->ee.ee_errno = ENOMSG;
- serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
- skb->sk = NULL;
-
- err = sock_queue_err_skb(sk, skb);
-
- sock_put(sk);
- if (err)
- kfree_skb(skb);
-}
-EXPORT_SYMBOL_GPL(skb_complete_tx_timestamp);
-
bool skb_defer_rx_timestamp(struct sk_buff *skb)
{
struct phy_device *phydev;
^ permalink raw reply related
* [PATCH net-next v4 2/3] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Alexander Duyck @ 2014-09-04 17:31 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140904172906.7702.87598.stgit@ahduyck-bv4.jf.intel.com>
The phy timestamping takes a different path than the regular timestamping
does in that it will create a clone first so that the packets needing to be
timestamped can be placed in a queue, or the context block could be used.
In order to support these use cases I am pulling the core of the code out
so it can be used in other drivers beyond just phy devices.
In addition I have added a destructor named sock_efree which is meant to
provide a simple way for dropping the reference to skb exceptions that
aren't part of either the receive or send windows for the socket, and I
have removed some duplication in spots where this destructor could be used
in place of sock_edemux.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
v2: Renamed function to skb_clone_sk.
Added destructor to call sock_put instead of doing it ourselves.
Dropped freeing functionality from skb_complete_tx_timestamp.
Added additional documentation to the code.
v3: Renamed destructor sock_efree and moved to sock.c/h
Added sock_hold/sock_put around call to sock_queue_err_skb
v4: Dropped combining sock_edemux with sock_efree where the 2 are identical
drivers/net/phy/dp83640.c | 6 +++---
include/linux/skbuff.h | 2 ++
include/net/sock.h | 1 +
net/core/skbuff.c | 32 +++++++++++++++++++++++++-------
net/core/sock.c | 6 ++++++
net/core/timestamping.c | 14 +++-----------
6 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index d5991ac..87648b3 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1148,7 +1148,7 @@ static void dp83640_remove(struct phy_device *phydev)
kfree_skb(skb);
while ((skb = skb_dequeue(&dp83640->tx_queue)) != NULL)
- skb_complete_tx_timestamp(skb, NULL);
+ kfree_skb(skb);
clock = dp83640_clock_get(dp83640->clock);
@@ -1405,7 +1405,7 @@ static void dp83640_txtstamp(struct phy_device *phydev,
case HWTSTAMP_TX_ONESTEP_SYNC:
if (is_sync(skb, type)) {
- skb_complete_tx_timestamp(skb, NULL);
+ kfree_skb(skb);
return;
}
/* fall through */
@@ -1416,7 +1416,7 @@ static void dp83640_txtstamp(struct phy_device *phydev,
case HWTSTAMP_TX_OFF:
default:
- skb_complete_tx_timestamp(skb, NULL);
+ kfree_skb(skb);
break;
}
}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 02529fc..1cf0cfa 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2690,6 +2690,8 @@ static inline ktime_t net_invalid_timestamp(void)
return ktime_set(0, 0);
}
+struct sk_buff *skb_clone_sk(struct sk_buff *skb);
+
#ifdef CONFIG_NETWORK_PHY_TIMESTAMPING
void skb_clone_tx_timestamp(struct sk_buff *skb);
diff --git a/include/net/sock.h b/include/net/sock.h
index 3fde613..e02be37 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1574,6 +1574,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force,
void sock_wfree(struct sk_buff *skb);
void skb_orphan_partial(struct sk_buff *skb);
void sock_rfree(struct sk_buff *skb);
+void sock_efree(struct sk_buff *skb);
void sock_edemux(struct sk_buff *skb);
int sock_setsockopt(struct socket *sock, int level, int op,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 697e696..a936a40 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3511,6 +3511,27 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
}
EXPORT_SYMBOL(sock_dequeue_err_skb);
+struct sk_buff *skb_clone_sk(struct sk_buff *skb)
+{
+ struct sock *sk = skb->sk;
+ struct sk_buff *clone;
+
+ if (!sk || !atomic_inc_not_zero(&sk->sk_refcnt))
+ return NULL;
+
+ clone = skb_clone(skb, GFP_ATOMIC);
+ if (!clone) {
+ sock_put(sk);
+ return NULL;
+ }
+
+ clone->sk = sk;
+ clone->destructor = sock_efree;
+
+ return clone;
+}
+EXPORT_SYMBOL(skb_clone_sk);
+
static void __skb_complete_tx_timestamp(struct sk_buff *skb,
struct sock *sk,
int tstype)
@@ -3540,14 +3561,11 @@ void skb_complete_tx_timestamp(struct sk_buff *skb,
{
struct sock *sk = skb->sk;
- skb->sk = NULL;
+ /* take a reference to prevent skb_orphan() from freeing the socket */
+ sock_hold(sk);
- if (hwtstamps) {
- *skb_hwtstamps(skb) = *hwtstamps;
- __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
- } else {
- kfree_skb(skb);
- }
+ *skb_hwtstamps(skb) = *hwtstamps;
+ __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
sock_put(sk);
}
diff --git a/net/core/sock.c b/net/core/sock.c
index f1a638e..d04005c 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1637,6 +1637,12 @@ void sock_rfree(struct sk_buff *skb)
}
EXPORT_SYMBOL(sock_rfree);
+void sock_efree(struct sk_buff *skb)
+{
+ sock_put(skb->sk);
+}
+EXPORT_SYMBOL(sock_efree);
+
void sock_edemux(struct sk_buff *skb)
{
struct sock *sk = skb->sk;
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index f48a59f..43d3dd6 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -36,10 +36,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
{
struct phy_device *phydev;
struct sk_buff *clone;
- struct sock *sk = skb->sk;
unsigned int type;
- if (!sk)
+ if (!skb->sk)
return;
type = classify(skb);
@@ -48,16 +47,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
phydev = skb->dev->phydev;
if (likely(phydev->drv->txtstamp)) {
- if (!atomic_inc_not_zero(&sk->sk_refcnt))
+ clone = skb_clone_sk(skb);
+ if (!clone)
return;
-
- clone = skb_clone(skb, GFP_ATOMIC);
- if (!clone) {
- sock_put(sk);
- return;
- }
-
- clone->sk = sk;
phydev->drv->txtstamp(phydev, clone, type);
}
}
^ permalink raw reply related
* [PATCH net-next v4 0/3] Series short description
From: Alexander Duyck @ 2014-09-04 17:30 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
This change makes it so that the core path for the phy timestamping logic
is shared between skb_tx_tstamp and skb_complete_tx_timestamp. In addition
it provides a means of using the same skb clone type path in non phy
timestamping drivers.
The main motivation for this is to enable non-phy drivers to be able to
manipulate tx timestamp skbs for such things as putting them in lists or
setting aside buffer in the context block.
---
v2: Incorporated suggested changes from Willem de Bruijn and Eric Dumazet
dropped uneeded comment
restored order of hwtstamp vs swtstamp
added destructor for skb
Dropped usage of skb_complete_tx_timestamp as a kfree_skb w/ destructor
v3: Updated destructor handling and dealt with socket reference counting issues
v4: Split out combining destructors into a separate patch
Alexander Duyck (3):
net-timestamp: Merge shared code between phy and regular timestamping
net-timestamp: Make the clone operation stand-alone from phy timestamping
net: merge cases where sock_efree and sock_edemux are the same function
drivers/net/phy/dp83640.c | 6 ++-
include/linux/skbuff.h | 2 +
include/net/sock.h | 5 +++
net/core/skbuff.c | 79 +++++++++++++++++++++++++++++++++------------
net/core/sock.c | 10 +++++-
net/core/timestamping.c | 43 ++----------------------
net/ipv4/udp.c | 2 +
7 files changed, 80 insertions(+), 67 deletions(-)
--
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Alexander Duyck @ 2014-09-04 17:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, richardcochran, davem, willemb
In-Reply-To: <1409849087.26422.124.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/04/2014 09:44 AM, Eric Dumazet wrote:
> On Thu, 2014-09-04 at 12:21 -0400, Alexander Duyck wrote:
>
>> In addition I have added a destructor named sock_efree which is meant to
>> provide a simple way for dropping the reference to skb exceptions that
>> aren't part of either the receive or send windows for the socket, and I
>> have removed some duplication in spots where this destructor could be used
>> in place of sock_edemux.
> This is an interesting idea but belongs to a separate patch ?
>
I suppose I can split it. Expect v4 shortly.
Thanks,
Alex
^ permalink raw reply
* Re: [RFC net-next PATCH V2 2/3] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Cong Wang @ 2014-09-04 17:09 UTC (permalink / raw)
To: Florian Westphal
Cc: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
Eric Dumazet, Hannes Frederic Sowa, Daniel Borkmann,
Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140904131702.GA934@breakpoint.cc>
On Thu, Sep 4, 2014 at 6:17 AM, Florian Westphal <fw@strlen.de> wrote:
>
> Unrelated to your patch: q->gso_skb should be renamed.
> It has nothing to do with gso anymore...
>
I have a pending patch which renames it to ->dequeued_skb.
^ permalink raw reply
* Re: [RFC net-next PATCH V2 2/3] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Tom Herbert @ 2014-09-04 16:59 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Linux Netdev List, David S. Miller, Eric Dumazet,
Hannes Frederic Sowa, Florian Westphal, Daniel Borkmann,
Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140904125438.4108.38160.stgit@dragon>
On Thu, Sep 4, 2014 at 5:55 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Based on DaveM's recent API work on dev_hard_start_xmit(), that allows
> sending/processing an entire skb list.
>
> This patch implements qdisc bulk dequeue, by allowing multiple packets
> to be dequeued in dequeue_skb().
>
> The optimization principle for this is two fold, (1) to amortize
> locking cost and (2) avoid expensive tailptr update for notifying HW.
> (1) Several packets are dequeued while holding the qdisc root_lock,
> amortizing locking cost over several packet. The dequeued SKB list is
> processed under the TXQ lock in dev_hard_start_xmit(), thus also
> amortizing the cost of the TXQ lock.
> (2) Further more, dev_hard_start_xmit() will utilize the skb->xmit_more
> API to delay HW tailptr update, which also reduces the cost per
> packet.
>
> One restriction of the new API is that every SKB must belong to the
> same TXQ. This patch takes the easy way out, by restricting bulk
> dequeue to qdisc's with the TCQ_F_ONETXQUEUE flag, that specifies the
> qdisc only have attached a single TXQ.
>
> Some detail about the flow; dev_hard_start_xmit() will process the skb
> list, and transmit packets individually towards the driver (see
> xmit_one()). In case the driver stops midway in the list, the
> remaining skb list is returned by dev_hard_start_xmit(). In
> sch_direct_xmit() this returned list is requeued by dev_requeue_skb().
>
> The patch also tries to limit the amount of bytes dequeued, based on
> the drivers BQL limits. It also tries to avoid and stop dequeuing
> when seeing a GSO packet (both real GSO and segmented GSO skb lists).
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> ---
> V2:
> - Restruct functions, split out functionality
> - Use BQL bytelimit to avoid overshooting driver limits, causing
> too large skb lists to be sitting on the requeue gso_skb.
>
> net/sched/sch_generic.c | 67 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 19696eb..a0c8070 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -56,6 +56,67 @@ static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
> return 0;
> }
>
> +static inline bool qdisc_may_bulk(const struct Qdisc *qdisc,
> + const struct sk_buff *skb)
> +{
> + return (qdisc->flags & TCQ_F_ONETXQUEUE);
> +}
> +
> +static inline struct sk_buff *qdisc_dequeue_validate(struct Qdisc *qdisc)
> +{
> + struct sk_buff *skb = qdisc->dequeue(qdisc);
> +
> + if (skb != NULL)
> + skb = validate_xmit_skb(skb, qdisc_dev(qdisc));
> +
> + return skb;
> +}
> +
> +static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
> + struct sk_buff *head)
> +{
> + struct sk_buff *new, *skb = head;
> + struct netdev_queue *txq = q->dev_queue;
> + int bytelimit = netdev_tx_avail_queue(txq);
> + int limit = 5;
> +
> + if (bytelimit <= 0)
> + return head;
> +
This should be "if (bytelimit < 0)" since BQL doesn't stop a queue
until available space goes negative. Also, I don't think this check
might not be needed anyway. I believe this function would only be
called when the queue is not stopped, so we can, and probably should,
send at least one packet to avoid the possibility of locking transmit.
> + do {
> + if (skb->next || skb_is_gso(skb)) {
> + /* Stop processing if the skb is already a skb
> + * list (e.g a segmented GSO packet) or a real
> + * GSO packet */
> + break;
> + }
> + new = qdisc_dequeue_validate(q);
> + if (new) {
> + skb->next = new;
> + skb = new;
> + bytelimit -= skb->len;
> + cnt++;
> + /* One problem here is it is difficult to
> + * requeue the "new" dequeued skb, e.g. in
> + * case of GSO, thus a "normal" packet can
> + * have a GSO packet on its ->next ptr.
> + *
> + * Requeue is difficult because if requeuing
> + * on q->gso_skb, then a second requeue can
> + * happen from sch_direct_xmit e.g. if driver
> + * returns NETDEV_TX_BUSY, which would
> + * overwrite this requeue.
> + */
Maybe we could just add another pointer e.g. hold_skb and use that to
requeue normal packets and continue to use gso_skb for GSO requeue? In
dequeue_skb, check gso_skb first, then hold_skb, and the
qdisc->dequeue.
> + }
> + } while (new && --limit && (bytelimit > 0));
Should be "bytelimit >= 0".
> + skb = head;
> +
> + return skb;
> +}
> +
> +/* Note that dequeue_skb can possibly return a SKB list (via skb->next).
> + * A requeued skb (via q->gso_skb) can also be a SKB list.
> + */
> static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
> {
> struct sk_buff *skb = q->gso_skb;
> @@ -71,9 +132,9 @@ static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
> skb = NULL;
> } else {
> if (!(q->flags & TCQ_F_ONETXQUEUE) || !netif_xmit_frozen_or_stopped(txq)) {
> - skb = q->dequeue(q);
> - if (skb)
> - skb = validate_xmit_skb(skb, qdisc_dev(q));
> + skb = qdisc_dequeue_validate(q);
> + if (skb && qdisc_may_bulk(q, skb))
> + skb = qdisc_bulk_dequeue_skb(q, skb);
> }
> }
>
>
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Eric Dumazet @ 2014-09-04 16:44 UTC (permalink / raw)
To: Alexander Duyck; +Cc: netdev, richardcochran, davem, willemb
In-Reply-To: <20140904162039.6883.39277.stgit@ahduyck-bv4.jf.intel.com>
On Thu, 2014-09-04 at 12:21 -0400, Alexander Duyck wrote:
> In addition I have added a destructor named sock_efree which is meant to
> provide a simple way for dropping the reference to skb exceptions that
> aren't part of either the receive or send windows for the socket, and I
> have removed some duplication in spots where this destructor could be used
> in place of sock_edemux.
This is an interesting idea but belongs to a separate patch ?
^ permalink raw reply
* [PATCH net-next v3 0/2] Combine standard and phy timestamping logic
From: Alexander Duyck @ 2014-09-04 16:20 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
This change makes it so that the core path for the phy timestamping logic
is shared between skb_tx_tstamp and skb_complete_tx_timestamp. In addition
it provides a means of using the same skb clone type path in non phy
timestamping drivers.
The main motivation for this is to enable non-phy drivers to be able to
manipulate tx timestamp skbs for such things as putting them in lists or
setting aside buffer in the context block.
---
v2: Incorporated suggested changes from Willem de Bruijn and Eric Dumazet
dropped uneeded comment
restored order of hwtstamp vs swtstamp
added destructor for skb
Dropped usage of skb_complete_tx_timestamp as a kfree_skb w/ destructor
v3: Updated destructor handling and dealt with socket reference counting issues
Alexander Duyck (2):
net-timestamp: Merge shared code between phy and regular timestamping
net-timestamp: Make the clone operation stand-alone from phy timestamping
drivers/net/phy/dp83640.c | 6 ++-
include/linux/skbuff.h | 2 +
include/net/sock.h | 5 +++
net/core/skbuff.c | 79 +++++++++++++++++++++++++++++++++------------
net/core/sock.c | 10 +++++-
net/core/timestamping.c | 43 ++----------------------
net/ipv4/udp.c | 2 +
7 files changed, 80 insertions(+), 67 deletions(-)
--
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Scott Feldman @ 2014-09-04 16:30 UTC (permalink / raw)
To: Simon Horman
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw, Jiri Pirko,
roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
netdev-u79uwXL29TY76Z2rM5mHXA,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20140904090447.GB3176-IxS8c3vjKQDk1uMJSBkQmQ@public.gmane.org>
On Sep 4, 2014, at 2:04 AM, Simon Horman <simon.horman-wFxRvT7yatFl57MIdRCFDg@public.gmane.org> wrote:
>
>
> [snip]
>
> In relation to ports and datapaths it seems to me that the API that
> has been developed accommodates a model where a port may belong to a
> switch device; that this topology is fixed before any API calls are made
> and that all all ports belonging to the same switch belong to the same
> datapath.
>
> This makes sense in the case of hardware that looks a lot like a switch.
> But I think that other scenarios are possible. For example hardware that
> is able to handle the same abstractions handled by the datapath: datapaths
> may be created or destroyed; vports may be added and removed from datapaths.
>
> So one might have a piece of hardware that is configured with more than one
> datapath configured and its different ports belong to it might be
> associated with different data paths.
I’ve tested multiple datapaths on one switch hardware with the current patch set and it works fine, without the need to push down any datapath id in the API. It works because a switch port can’t belong to more than one datapath. Datapaths can be created/destroyed and ports added/removed from datapaths dynamically and the right sw_flows are added/removed to program HW.
>
> Or we might have hardware that is able to offload a tunnel vport.
>
> In short I am thinking in terms of API callbacks to manipulate datapaths
> and vports. Although I have not thought about it in detail I believe
> that the current model you have implemented using such a scheme because
> the scheme I am suggesting maps to that of the datapath and you have
> implemented your model there.
-scott
^ permalink raw reply
* [PATCH net-next v3 1/2] net-timestamp: Merge shared code between phy and regular timestamping
From: Alexander Duyck @ 2014-09-04 16:20 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140904161314.6883.22217.stgit@ahduyck-bv4.jf.intel.com>
This change merges the shared bits that exist between skb_tx_tstamp and
skb_complete_tx_timestamp. By doing this we can avoid the two diverging as
there were already changes pushed into skb_tx_tstamp that hadn't made it
into the other function.
In addition this resolves issues with the fact that
skb_complete_tx_timestamp was included in linux/skbuff.h even though it was
only compiled in if phy timestamping was enabled.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
v2: Dropped comment and restored original order based on comments
from Willem de Bruijn.
v3: No changes.
net/core/skbuff.c | 65 ++++++++++++++++++++++++++++++-----------------
net/core/timestamping.c | 29 ---------------------
2 files changed, 42 insertions(+), 52 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 53ce536..697e696 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3511,33 +3511,13 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
}
EXPORT_SYMBOL(sock_dequeue_err_skb);
-void __skb_tstamp_tx(struct sk_buff *orig_skb,
- struct skb_shared_hwtstamps *hwtstamps,
- struct sock *sk, int tstype)
+static void __skb_complete_tx_timestamp(struct sk_buff *skb,
+ struct sock *sk,
+ int tstype)
{
struct sock_exterr_skb *serr;
- struct sk_buff *skb;
int err;
- if (!sk)
- return;
-
- if (hwtstamps) {
- *skb_hwtstamps(orig_skb) =
- *hwtstamps;
- } else {
- /*
- * no hardware time stamps available,
- * so keep the shared tx_flags and only
- * store software time stamp
- */
- orig_skb->tstamp = ktime_get_real();
- }
-
- skb = skb_clone(orig_skb, GFP_ATOMIC);
- if (!skb)
- return;
-
serr = SKB_EXT_ERR(skb);
memset(serr, 0, sizeof(*serr));
serr->ee.ee_errno = ENOMSG;
@@ -3554,6 +3534,45 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
if (err)
kfree_skb(skb);
}
+
+void skb_complete_tx_timestamp(struct sk_buff *skb,
+ struct skb_shared_hwtstamps *hwtstamps)
+{
+ struct sock *sk = skb->sk;
+
+ skb->sk = NULL;
+
+ if (hwtstamps) {
+ *skb_hwtstamps(skb) = *hwtstamps;
+ __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
+ } else {
+ kfree_skb(skb);
+ }
+
+ sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(skb_complete_tx_timestamp);
+
+void __skb_tstamp_tx(struct sk_buff *orig_skb,
+ struct skb_shared_hwtstamps *hwtstamps,
+ struct sock *sk, int tstype)
+{
+ struct sk_buff *skb;
+
+ if (!sk)
+ return;
+
+ if (hwtstamps)
+ *skb_hwtstamps(orig_skb) = *hwtstamps;
+ else
+ orig_skb->tstamp = ktime_get_real();
+
+ skb = skb_clone(orig_skb, GFP_ATOMIC);
+ if (!skb)
+ return;
+
+ __skb_complete_tx_timestamp(skb, sk, tstype);
+}
EXPORT_SYMBOL_GPL(__skb_tstamp_tx);
void skb_tstamp_tx(struct sk_buff *orig_skb,
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index a877039..f48a59f 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -63,35 +63,6 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(skb_clone_tx_timestamp);
-void skb_complete_tx_timestamp(struct sk_buff *skb,
- struct skb_shared_hwtstamps *hwtstamps)
-{
- struct sock *sk = skb->sk;
- struct sock_exterr_skb *serr;
- int err;
-
- if (!hwtstamps) {
- sock_put(sk);
- kfree_skb(skb);
- return;
- }
-
- *skb_hwtstamps(skb) = *hwtstamps;
-
- serr = SKB_EXT_ERR(skb);
- memset(serr, 0, sizeof(*serr));
- serr->ee.ee_errno = ENOMSG;
- serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
- skb->sk = NULL;
-
- err = sock_queue_err_skb(sk, skb);
-
- sock_put(sk);
- if (err)
- kfree_skb(skb);
-}
-EXPORT_SYMBOL_GPL(skb_complete_tx_timestamp);
-
bool skb_defer_rx_timestamp(struct sk_buff *skb)
{
struct phy_device *phydev;
^ permalink raw reply related
* [PATCH net-next v3 2/2] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Alexander Duyck @ 2014-09-04 16:21 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140904161314.6883.22217.stgit@ahduyck-bv4.jf.intel.com>
The phy timestamping takes a different path than the regular timestamping
does in that it will create a clone first so that the packets needing to be
timestamped can be placed in a queue, or the context block could be used.
In order to support these use cases I am pulling the core of the code out
so it can be used in other drivers beyond just phy devices.
In addition I have added a destructor named sock_efree which is meant to
provide a simple way for dropping the reference to skb exceptions that
aren't part of either the receive or send windows for the socket, and I
have removed some duplication in spots where this destructor could be used
in place of sock_edemux.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
v2: Renamed function to skb_clone_sk.
Added destructor to call sock_put instead of doing it ourselves.
Dropped freeing functionality from skb_complete_tx_timestamp.
Added additional documentation to the code.
v3: Renamed destructor sock_efree and moved to sock.c/h
Combined sock_edemux with sock_efree where the 2 are identical
Added sock_hold/sock_put around call to sock_queue_err_skb
drivers/net/phy/dp83640.c | 6 +++---
include/linux/skbuff.h | 2 ++
include/net/sock.h | 5 +++++
net/core/skbuff.c | 32 +++++++++++++++++++++++++-------
net/core/sock.c | 10 ++++++++--
net/core/timestamping.c | 14 +++-----------
net/ipv4/udp.c | 2 +-
7 files changed, 47 insertions(+), 24 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index d5991ac..87648b3 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1148,7 +1148,7 @@ static void dp83640_remove(struct phy_device *phydev)
kfree_skb(skb);
while ((skb = skb_dequeue(&dp83640->tx_queue)) != NULL)
- skb_complete_tx_timestamp(skb, NULL);
+ kfree_skb(skb);
clock = dp83640_clock_get(dp83640->clock);
@@ -1405,7 +1405,7 @@ static void dp83640_txtstamp(struct phy_device *phydev,
case HWTSTAMP_TX_ONESTEP_SYNC:
if (is_sync(skb, type)) {
- skb_complete_tx_timestamp(skb, NULL);
+ kfree_skb(skb);
return;
}
/* fall through */
@@ -1416,7 +1416,7 @@ static void dp83640_txtstamp(struct phy_device *phydev,
case HWTSTAMP_TX_OFF:
default:
- skb_complete_tx_timestamp(skb, NULL);
+ kfree_skb(skb);
break;
}
}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 02529fc..1cf0cfa 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2690,6 +2690,8 @@ static inline ktime_t net_invalid_timestamp(void)
return ktime_set(0, 0);
}
+struct sk_buff *skb_clone_sk(struct sk_buff *skb);
+
#ifdef CONFIG_NETWORK_PHY_TIMESTAMPING
void skb_clone_tx_timestamp(struct sk_buff *skb);
diff --git a/include/net/sock.h b/include/net/sock.h
index 3fde613..ad23e80 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1574,7 +1574,12 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force,
void sock_wfree(struct sk_buff *skb);
void skb_orphan_partial(struct sk_buff *skb);
void sock_rfree(struct sk_buff *skb);
+void sock_efree(struct sk_buff *skb);
+#ifdef CONFIG_INET
void sock_edemux(struct sk_buff *skb);
+#else
+#define sock_edemux(skb) sock_efree(skb)
+#endif
int sock_setsockopt(struct socket *sock, int level, int op,
char __user *optval, unsigned int optlen);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 697e696..a936a40 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3511,6 +3511,27 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
}
EXPORT_SYMBOL(sock_dequeue_err_skb);
+struct sk_buff *skb_clone_sk(struct sk_buff *skb)
+{
+ struct sock *sk = skb->sk;
+ struct sk_buff *clone;
+
+ if (!sk || !atomic_inc_not_zero(&sk->sk_refcnt))
+ return NULL;
+
+ clone = skb_clone(skb, GFP_ATOMIC);
+ if (!clone) {
+ sock_put(sk);
+ return NULL;
+ }
+
+ clone->sk = sk;
+ clone->destructor = sock_efree;
+
+ return clone;
+}
+EXPORT_SYMBOL(skb_clone_sk);
+
static void __skb_complete_tx_timestamp(struct sk_buff *skb,
struct sock *sk,
int tstype)
@@ -3540,14 +3561,11 @@ void skb_complete_tx_timestamp(struct sk_buff *skb,
{
struct sock *sk = skb->sk;
- skb->sk = NULL;
+ /* take a reference to prevent skb_orphan() from freeing the socket */
+ sock_hold(sk);
- if (hwtstamps) {
- *skb_hwtstamps(skb) = *hwtstamps;
- __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
- } else {
- kfree_skb(skb);
- }
+ *skb_hwtstamps(skb) = *hwtstamps;
+ __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
sock_put(sk);
}
diff --git a/net/core/sock.c b/net/core/sock.c
index f1a638e..69592cb 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1637,18 +1637,24 @@ void sock_rfree(struct sk_buff *skb)
}
EXPORT_SYMBOL(sock_rfree);
+void sock_efree(struct sk_buff *skb)
+{
+ sock_put(skb->sk);
+}
+EXPORT_SYMBOL(sock_efree);
+
+#ifdef CONFIG_INET
void sock_edemux(struct sk_buff *skb)
{
struct sock *sk = skb->sk;
-#ifdef CONFIG_INET
if (sk->sk_state == TCP_TIME_WAIT)
inet_twsk_put(inet_twsk(sk));
else
-#endif
sock_put(sk);
}
EXPORT_SYMBOL(sock_edemux);
+#endif
kuid_t sock_i_uid(struct sock *sk)
{
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index f48a59f..43d3dd6 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -36,10 +36,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
{
struct phy_device *phydev;
struct sk_buff *clone;
- struct sock *sk = skb->sk;
unsigned int type;
- if (!sk)
+ if (!skb->sk)
return;
type = classify(skb);
@@ -48,16 +47,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
phydev = skb->dev->phydev;
if (likely(phydev->drv->txtstamp)) {
- if (!atomic_inc_not_zero(&sk->sk_refcnt))
+ clone = skb_clone_sk(skb);
+ if (!clone)
return;
-
- clone = skb_clone(skb, GFP_ATOMIC);
- if (!clone) {
- sock_put(sk);
- return;
- }
-
- clone->sk = sk;
phydev->drv->txtstamp(phydev, clone, type);
}
}
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0da3849..cd0db54 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1972,7 +1972,7 @@ void udp_v4_early_demux(struct sk_buff *skb)
return;
skb->sk = sk;
- skb->destructor = sock_edemux;
+ skb->destructor = sock_efree;
dst = sk->sk_rx_dst;
if (dst)
^ permalink raw reply related
* [PATCH net-next] ipv4: harden fnhe_hashfun()
From: Eric Dumazet @ 2014-09-04 15:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell
From: Eric Dumazet <edumazet@google.com>
Lets make this hash function a bit secure, as ICMP attacks are still
in the wild.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip_fib.h | 3 ++-
net/ipv4/route.c | 8 ++++----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 9922093f575e..6c8f00236a47 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -65,7 +65,8 @@ struct fnhe_hash_bucket {
struct fib_nh_exception __rcu *chain;
};
-#define FNHE_HASH_SIZE 2048
+#define FNHE_HASH_SHIFT 11
+#define FNHE_HASH_SIZE (1 << FNHE_HASH_SHIFT)
#define FNHE_RECLAIM_DEPTH 5
struct fib_nh {
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index eaa4b000c7b4..deb7a2ec3669 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -596,12 +596,12 @@ static struct fib_nh_exception *fnhe_oldest(struct fnhe_hash_bucket *hash)
static inline u32 fnhe_hashfun(__be32 daddr)
{
+ static u32 fnhe_hashrnd __read_mostly;
u32 hval;
- hval = (__force u32) daddr;
- hval ^= (hval >> 11) ^ (hval >> 22);
-
- return hval & (FNHE_HASH_SIZE - 1);
+ net_get_random_once(&fnhe_hashrnd, sizeof(fnhe_hashrnd));
+ hval = jhash_1word((__force u32) daddr, fnhe_hashrnd);
+ return hash_32(hval, FNHE_HASH_SHIFT);
}
static void fill_route_from_fnhe(struct rtable *rt, struct fib_nh_exception *fnhe)
^ permalink raw reply related
* Does NetworkManager kill IPv6 connections?
From: David Laight @ 2014-09-04 14:47 UTC (permalink / raw)
To: netdev@vger.kernel.org
I was running a overnight test that failed because an (almost idle) IPv6 connection
unexpectedly disconnected.
30 seconds before the disconnect syslog contains (retyped):
NetworkManager[pid]: <info> policy set 'Wired connection 2' (eth0) as default for IPv6 routing and DNS.
The same message appeared exactly 70 minutes earlier - with no ill effect.
A second connection between the same pair if addresses (carrying a lot of traffic)
was unaffected.
Any idea what/why NetworkManager is doing?
(Yes I know it is userspace, but it seems to be breaking the kernel....)
The system is an ubuntu 13.04 desktop (3.8.0-19 kernel), that recent, but
newer than the ones a lot of our customers use.
I can rerun the test, but don't know what to look for.
'eth0' is the only ethernet interface in the system that is actually
connected to a network.
Some indications of where to look would be appreciated.
David
^ permalink raw reply
* [PATCH/v2] net: treewide: Fix typo found in DocBook/networking.xml
From: Masanari Iida @ 2014-09-04 14:44 UTC (permalink / raw)
To: linux-kernel, rdunlap, netdev, davem, linux-doc, trivial; +Cc: Masanari Iida
In-Reply-To: <20140903.203814.642492840095995090.davem@davemloft.net>
This patch fix spelling typo found in DocBook/networking.xml.
It is because the neworking.xml is generated from comments
in the source, I have to fix typo in comments within the source.
Signed-off-by: Masanari Iida <standby24x7@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
---
include/linux/netdevice.h | 4 ++--
include/net/wimax.h | 2 +-
include/trace/events/irq.h | 4 ++--
net/core/datagram.c | 2 +-
net/core/gen_estimator.c | 2 +-
net/core/gen_stats.c | 2 +-
net/core/skbuff.c | 4 ++--
net/core/sock.c | 4 ++--
net/socket.c | 2 +-
9 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 039b237..d2b1704 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3249,7 +3249,7 @@ static inline int __dev_uc_sync(struct net_device *dev,
}
/**
- * __dev_uc_unsync - Remove synchonized addresses from device
+ * __dev_uc_unsync - Remove synchronized addresses from device
* @dev: device to sync
* @unsync: function to call if address should be removed
*
@@ -3293,7 +3293,7 @@ static inline int __dev_mc_sync(struct net_device *dev,
}
/**
- * __dev_mc_unsync - Remove synchonized addresses from device
+ * __dev_mc_unsync - Remove synchronized addresses from device
* @dev: device to sync
* @unsync: function to call if address should be removed
*
diff --git a/include/net/wimax.h b/include/net/wimax.h
index e52ef53..c52b685 100644
--- a/include/net/wimax.h
+++ b/include/net/wimax.h
@@ -290,7 +290,7 @@ struct wimax_dev;
* This operation has to be synchronous, and return only when the
* reset is complete. In case of having had to resort to bus/cold
* reset implying a device disconnection, the call is allowed to
- * return inmediately.
+ * return immediately.
* NOTE: wimax_dev->mutex is NOT locked when this op is being
* called; however, wimax_dev->mutex_reset IS locked to ensure
* serialization of calls to wimax_reset().
diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
index 1c09820..3608beb 100644
--- a/include/trace/events/irq.h
+++ b/include/trace/events/irq.h
@@ -107,7 +107,7 @@ DECLARE_EVENT_CLASS(softirq,
* @vec_nr: softirq vector number
*
* When used in combination with the softirq_exit tracepoint
- * we can determine the softirq handler runtine.
+ * we can determine the softirq handler routine.
*/
DEFINE_EVENT(softirq, softirq_entry,
@@ -121,7 +121,7 @@ DEFINE_EVENT(softirq, softirq_entry,
* @vec_nr: softirq vector number
*
* When used in combination with the softirq_entry tracepoint
- * we can determine the softirq handler runtine.
+ * we can determine the softirq handler routine.
*/
DEFINE_EVENT(softirq, softirq_exit,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 488dd1a..fdbc9a8 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -775,7 +775,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
EXPORT_SYMBOL(__skb_checksum_complete);
/**
- * skb_copy_and_csum_datagram_iovec - Copy and checkum skb to user iovec.
+ * skb_copy_and_csum_datagram_iovec - Copy and checksum skb to user iovec.
* @skb: skbuff
* @hlen: hardware length
* @iov: io vector
diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 6b5b6e7..9d33dff 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -197,7 +197,7 @@ struct gen_estimator *gen_find_node(const struct gnet_stats_basic_packed *bstats
* as destination. A new timer with the interval specified in the
* configuration TLV is created. Upon each interval, the latest statistics
* will be read from &bstats and the estimated rate will be stored in
- * &rate_est with the statistics lock grabed during this period.
+ * &rate_est with the statistics lock grabbed during this period.
*
* Returns 0 on success or a negative error code.
*
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 9d3d9e7..2ddbce4 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -206,7 +206,7 @@ EXPORT_SYMBOL(gnet_stats_copy_queue);
* @st: application specific statistics data
* @len: length of data
*
- * Appends the application sepecific statistics to the top level TLV created by
+ * Appends the application specific statistics to the top level TLV created by
* gnet_stats_start_copy() and remembers the data for XSTATS if the dumping
* handle is in backward compatibility mode.
*
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index adfc7ee..2d9f117 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2647,7 +2647,7 @@ EXPORT_SYMBOL(skb_prepare_seq_read);
* skb_seq_read() will return the remaining part of the block.
*
* Note 1: The size of each block of data returned can be arbitrary,
- * this limitation is the cost for zerocopy seqeuental
+ * this limitation is the cost for zerocopy sequential
* reads of potentially non linear data.
*
* Note 2: Fragment lists within fragments are not implemented
@@ -2781,7 +2781,7 @@ EXPORT_SYMBOL(skb_find_text);
/**
* skb_append_datato_frags - append the user data to a skb
* @sk: sock structure
- * @skb: skb structure to be appened with user data.
+ * @skb: skb structure to be appended with user data.
* @getfrag: call back function to be used for getting the user data
* @from: pointer to user message iov
* @length: length of the iov message
diff --git a/net/core/sock.c b/net/core/sock.c
index f7f2352..8bde925 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -166,7 +166,7 @@ EXPORT_SYMBOL(sk_ns_capable);
/**
* sk_capable - Socket global capability test
* @sk: Socket to use a capability on or through
- * @cap: The global capbility to use
+ * @cap: The global capability to use
*
* Test to see if the opener of the socket had when the socket was
* created and the current process has the capability @cap in all user
@@ -183,7 +183,7 @@ EXPORT_SYMBOL(sk_capable);
* @sk: Socket to use a capability on or through
* @cap: The capability to use
*
- * Test to see if the opener of the socket had when the socke was created
+ * Test to see if the opener of the socket had when the socket was created
* and the current process has the capability @cap over the network namespace
* the socket is a member of.
*/
diff --git a/net/socket.c b/net/socket.c
index 95ee7d8..0593ebd 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2602,7 +2602,7 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
*
* This function is called by a protocol handler that wants to
* advertise its address family, and have it linked into the
- * socket interface. The value ops->family coresponds to the
+ * socket interface. The value ops->family corresponds to the
* socket system call protocol family.
*/
int sock_register(const struct net_proto_family *ops)
--
2.1.0.24.g4109c28
^ 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