* Re: [PATCH 2/2] [sky2] Fix for interrupt handler
From: Jonathan Nieder @ 2012-10-25 22:36 UTC (permalink / raw)
To: David S. Miller; +Cc: Julian Gilbey, Mirko Lindner, Stephen Hemminger, netdev
In-Reply-To: <1341394726.14972.41.camel@mlindner-lin.skd.de>
Hi,
Mirko Lindner wrote:
> Re-enable interrupts if it is not our interrupt
>
> Signed-off-by: Mirko Lindner <mlindner@marvell.com>
Julian (cc-ed) is experiencing tx stalls[1] on a 3.2.y-based kernel:
WARNING: at [...]/linux-3.2.21/net/sched/sch_generic.c:255 dev_watchdog+0xe9/0x148()
Hardware name: Inspiron 1545
NETDEV WATCHDOG: eth0 (sky2): transmit queue 0 timed out
Cherry-picking v3.6-rc1~125^2~264 (sky2: Fix for interrupt handler,
2012-07-03) fixes it.
Tested-by: Julian Gilbey <jdg@debian.org>
Dave, would this be a candidate for stable? Patch left unsnipped for
reference.
Thanks,
Jonathan
[1] http://bugs.debian.org/681280
[2] http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
> ---
> drivers/net/ethernet/marvell/sky2.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
> index f1163b2..2b0748d 100644
> --- a/drivers/net/ethernet/marvell/sky2.c
> +++ b/drivers/net/ethernet/marvell/sky2.c
> @@ -3080,8 +3080,10 @@ static irqreturn_t sky2_intr(int irq, void *dev_id)
>
> /* Reading this mask interrupts as side effect */
> status = sky2_read32(hw, B0_Y2_SP_ISRC2);
> - if (status == 0 || status == ~0)
> + if (status == 0 || status == ~0) {
> + sky2_write32(hw, B0_Y2_SP_ICR, 2);
> return IRQ_NONE;
> + }
>
> prefetch(&hw->st_le[hw->st_idx]);
>
> --
> 1.7.10.4
^ permalink raw reply
* Re: Disable tcp MSS option in three way handshake?
From: Eric Dumazet @ 2012-10-25 22:42 UTC (permalink / raw)
To: Vincent Li; +Cc: netdev@vger.kernel.org
In-Reply-To: <CAK3+h2xKHBNXzY0WNwPV+oGbY00UVfJpO-qjFW2cxCpmdt+hAg@mail.gmail.com>
On Thu, 2012-10-25 at 15:27 -0700, Vincent Li wrote:
> Hi,
>
> this sounds crazy, we have a weird situation that an unknown tcp
> implementation not putting tcp MSS option in the SYN/ACK which caused
> us some issue. I am tasked to mimic the unknown tcp immplementation on
> not sending MSS in tcp SYN/ACK, I am wondering if I can achieve that
> by modifying linux kernel tcp code, there is socket option
> TCP_MAXSEG, but that seems only affecting the size of MSS, not
> removing the MSS option. do you have any pointer on how to do that in
> kernel tcp code?
You'll have to patch the code.
Or else, you could add a new feature to net/netfilter/xt_TCPMSS.c
(We already have
#define XT_TCPMSS_CLAMP_PMTU 0xffff
You could add
#define XT_TCPMSS_REMOVE 0xfffe
and replace MSS option by NOP
^ permalink raw reply
* Re: listen(2) backlog changes in or around Linux 3.1?
From: Eric Dumazet @ 2012-10-25 22:50 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: enh, Venkat Venkatsubra, netdev
In-Reply-To: <CAGK4HS9N9b64pKobWE=9wM-ZqNL-uJiGoAWNkqu=F=-NgHM=0Q@mail.gmail.com>
On Mon, 2012-10-22 at 15:11 -0700, Vijay Subramanian wrote:
> >
> > All these facilities sound very complex and not really usable by clients
> > (ie users not willing to wait more than few seconds anyway)
> >
>
> Fair enough. We can drop this if it is not worth the trouble or if I
> have missed any other scenario.
>
Sorry my comment was not related to your patch, but existing logic.
It seems there is no value resending SYNACK, as we received the client
ACK.
Please send an official patch ?
^ permalink raw reply
* Re: Disable tcp MSS option in three way handshake?
From: Rick Jones @ 2012-10-25 22:50 UTC (permalink / raw)
To: Vincent Li, netdev
In-Reply-To: <CAK3+h2xKHBNXzY0WNwPV+oGbY00UVfJpO-qjFW2cxCpmdt+hAg@mail.gmail.com>
On 10/25/2012 03:27 PM, Vincent Li wrote:
> Hi,
>
> this sounds crazy, we have a weird situation that an unknown tcp
> implementation not putting tcp MSS option in the SYN/ACK which caused
> us some issue.
All that means is you/your TCP stack are to assume an MSS of 536 bytes.
In that sense at least, there is nothing (supposed to be) weird about it.
rick jones
^ permalink raw reply
* Re: Disable tcp MSS option in three way handshake?
From: Vincent Li @ 2012-10-25 22:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev@vger.kernel.org
In-Reply-To: <1351204948.6537.263.camel@edumazet-glaptop>
On Thu, Oct 25, 2012 at 3:42 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2012-10-25 at 15:27 -0700, Vincent Li wrote:
>> Hi,
>>
>> this sounds crazy, we have a weird situation that an unknown tcp
>> implementation not putting tcp MSS option in the SYN/ACK which caused
>> us some issue. I am tasked to mimic the unknown tcp immplementation on
>> not sending MSS in tcp SYN/ACK, I am wondering if I can achieve that
>> by modifying linux kernel tcp code, there is socket option
>> TCP_MAXSEG, but that seems only affecting the size of MSS, not
>> removing the MSS option. do you have any pointer on how to do that in
>> kernel tcp code?
>
> You'll have to patch the code.
>
> Or else, you could add a new feature to net/netfilter/xt_TCPMSS.c
>
> (We already have
>
> #define XT_TCPMSS_CLAMP_PMTU 0xffff
>
> You could add
>
> #define XT_TCPMSS_REMOVE 0xfffe
>
> and replace MSS option by NOP
>
>
>
thanks for the reply, I did a quick look at the code, as a quick dirty
hack, can I change the
699 /* Set up TCP options for SYN-ACKs. */
700 static unsigned int tcp_synack_options(struct sock *sk,
701 struct request_sock *req,
702 unsigned int mss, struct sk_buff *skb,
703 struct tcp_out_options *opts,
704 struct tcp_md5sig_key **md5,
705 struct tcp_extend_values *xvp)
706 {
..................
730 /* We always send an MSS option. */
731 opts->mss = mss; <---------here set opts->mss = 0 ?
would that work?
^ permalink raw reply
* Re: [PATCH 5/5] ss: Show inet and unix sockets' shutdown state
From: Stephen Hemminger @ 2012-10-25 23:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Pavel Emelyanov, Linux Netdev List
In-Reply-To: <1351188054.6537.196.camel@edumazet-glaptop>
On Thu, 25 Oct 2012 20:00:54 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2012-10-25 at 10:09 -0700, Stephen Hemminger wrote:
>
> > I don't want the stashed kernel headers to get out of date with kernel exported ones.
> >
> > People who want to test early features are free to build their own pre-release versions.
> > --
>
> Have you considered an iproute2-next tree, it would really help us a
> lot.
>
> Just to be clear, were the patches 1-4 accepted ?
>
> (UDP diag was added kernel in December 2011)
>
> Thanks
>
>
The first 4 patches are fine, and I will probably take those.
^ permalink raw reply
* Re: Disable tcp MSS option in three way handshake?
From: Vijay Subramanian @ 2012-10-25 23:15 UTC (permalink / raw)
To: Vincent Li; +Cc: Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <CAK3+h2zLGGA2w38mRwpf5Ww+d8ZdtESp1zix82OZODG7o=Rsbg@mail.gmail.com>
> thanks for the reply, I did a quick look at the code, as a quick dirty
> hack, can I change the
>
> 699 /* Set up TCP options for SYN-ACKs. */
> 700 static unsigned int tcp_synack_options(struct sock *sk,
> 701 struct request_sock *req,
> 702 unsigned int mss, struct sk_buff *skb,
> 703 struct tcp_out_options *opts,
> 704 struct tcp_md5sig_key **md5,
> 705 struct tcp_extend_values *xvp)
> 706 {
>
> ..................
> 730 /* We always send an MSS option. */
> 731 opts->mss = mss; <---------here set opts->mss = 0 ?
>
> would that work?
tcp_make_synack() calls tcp_options_write() has this:
if (unlikely(opts->mss)) {
*ptr++ = htonl((TCPOPT_MSS << 24) |
(TCPOLEN_MSS << 16) |
opts->mss);
}
It will not add the option if opts->mss is zero, so this should work too.
Vijay
^ permalink raw reply
* Re: listen(2) backlog changes in or around Linux 3.1?
From: Vijay Subramanian @ 2012-10-25 23:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: enh, Venkat Venkatsubra, netdev
In-Reply-To: <1351205417.6537.272.camel@edumazet-glaptop>
On 25 October 2012 15:50, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2012-10-22 at 15:11 -0700, Vijay Subramanian wrote:
>
>> >
>> > All these facilities sound very complex and not really usable by clients
>> > (ie users not willing to wait more than few seconds anyway)
>> >
>>
>> Fair enough. We can drop this if it is not worth the trouble or if I
>> have missed any other scenario.
>>
>
> Sorry my comment was not related to your patch, but existing logic.
>
> It seems there is no value resending SYNACK, as we received the client
> ACK.
>
> Please send an official patch ?
>
>
>
Eric,
I will send a patch shortly.
Thanks,
Vijay
^ permalink raw reply
* Re: [PATCH] sctp: Clean up type-punning in sctp_cmd_t union
From: Neil Horman @ 2012-10-25 23:58 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, David S. Miller, linux-sctp
In-Reply-To: <5089B237.6010902@gmail.com>
On Thu, Oct 25, 2012 at 05:42:15PM -0400, Vlad Yasevich wrote:
> On 10/25/2012 04:47 PM, Neil Horman wrote:
> >Lots of points in the sctp_cmd_interpreter function treat the sctp_cmd_t arg as
> >a void pointer, even though they are written as various other types. Theres no
> >need for this as doing so just leads to possible type-punning issues that could
> >cause crashes, and if we remain type-consistent we can actually just remove the
> >void * member of the union entirely.
> >
> >Signed-off-by: Neil Horman <nhorman@tuxdriver.com
> >CC: Vlad Yasevich <vyasevich@gmail.com>
> >CC: "David S. Miller" <davem@davemloft.net>
> >CC: linux-sctp@vger.kernel.org
> >---
> > include/net/sctp/command.h | 7 ++++---
> > include/net/sctp/ulpqueue.h | 2 +-
> > net/sctp/sm_sideeffect.c | 45 ++++++++++++++++++++++-----------------------
> > net/sctp/ulpqueue.c | 3 +--
> > 4 files changed, 28 insertions(+), 29 deletions(-)
> >
> >diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> >index 712b3be..7f1b0f3 100644
> >--- a/include/net/sctp/command.h
> >+++ b/include/net/sctp/command.h
> >@@ -131,7 +131,6 @@ typedef union {
> > sctp_state_t state;
> > sctp_event_timeout_t to;
> > unsigned long zero;
> >- void *ptr;
> > struct sctp_chunk *chunk;
> > struct sctp_association *asoc;
> > struct sctp_transport *transport;
> >@@ -154,9 +153,12 @@ typedef union {
> > * which takes an __s32 and returns a sctp_arg_t containing the
> > * __s32. So, after foo = SCTP_I32(arg), foo.i32 == arg.
> > */
> >+#define SCTP_NULL_BYTE 0xAA
> > static inline sctp_arg_t SCTP_NULL(void)
> > {
> >- sctp_arg_t retval; retval.ptr = NULL; return retval;
> >+ sctp_arg_t retval;
> >+ memset(&retval, SCTP_NULL_BYTE, sizeof(sctp_arg_t));
> >+ return retval;
>
> What's this for? Can't we just use retval.zero?
>
> -vlad
>
My intent was to highlight any users of sctp_arg_t when SCTP_NULL was passed.
My thinking was that the 0xAA byte patern would be a good indicator. Although,
admittedly I didn't see the zero argument there. Looking at it though, the zero
member of the union is effectively unused. Strictly speaking its used for
initalization of sctp_arg_t, but its done somewhat poorly, since theres no
guarantee that an unsigned long will be the largest member of that union. Doing
the memset guarantees the whole instance is set to a predefined value.
I could go either way with this, would you rather we just have SCTP_NULL return
retval = { .zero = 0}; or would you rather remove the zero initialization from
SCTP_[NO]FORCE, and SCTP_ARG_CONSTRUCTOR and do the memset. I think the memset
reduces to a single 64 bit assignment as long as the union doesn't exceed that
size anyway, and it ensures that you initalize the whole union's storage if it
does in the future. And if we remove the initialization step (I don't see that
its needed in the three macros above anyway), then we can remove the zero member
as well.
Let me know what you want to do here, and I can respin this.
Best
Neil
^ permalink raw reply
* [PATCH net-next 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Vijay Subramanian @ 2012-10-26 0:45 UTC (permalink / raw)
To: netdev
Cc: davem, Venkat Venkatsubra, Elliot Hughes, Vijay Subramanian,
Eric Dumazet
If server socket is slow to accept() connections, request_socks can represent
connections for which the three-way handshake is already done. From client's
point of view, the connection is in ESTABLISHED state but on server side, socket
is not in accept_queue or ESTABLISHED state. When the syn-ack timer expires,
because of the order in which tests are performed, server can retransmit the
synack repeatedly. Following patch prevents the server from retransmitting the
synack needlessly (and prevents client from replying with ack). This reduces
traffic when server is slow to accept() connections.
If the server socket has received the third ack during connection establishment,
this is remembered in inet_rsk(req)->acked. The request_sock will expire in
around 30 seconds and will be dropped if it does not move into accept_queue.
With help from Eric Dumazet.
Reported-by: Elliot Hughes <enh@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
Ignoring "WARNING: line over 80 characters" in the interest of readability.
Eric,
What about your earlier patch that modified sk_acceptq_is_full()?
net/ipv4/inet_connection_sock.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index d34ce29..4e8e52e 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -598,9 +598,8 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
&expire, &resend);
req->rsk_ops->syn_ack_timeout(parent, req);
if (!expire &&
- (!resend ||
- !req->rsk_ops->rtx_syn_ack(parent, req, NULL) ||
- inet_rsk(req)->acked)) {
+ (!resend || inet_rsk(req)->acked ||
+ !req->rsk_ops->rtx_syn_ack(parent, req, NULL))) {
unsigned long timeo;
if (req->retrans++ == 0)
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/1] af_unix.c: fix the compile warning in unix_bind
From: Jing Wang @ 2012-10-26 2:48 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Al Viro, Pavel Emelyanov; +Cc: netdev, Jing Wang
Signed-off-by: Jing Wang <windsdaemon@gmail.com>
---
net/unix/af_unix.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 5b5c876..3f7d9d5 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -860,6 +860,8 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
unsigned int hash;
struct unix_address *addr;
struct hlist_head *list;
+ struct path path;
+ umode_t mode;
err = -EINVAL;
if (sunaddr->sun_family != AF_UNIX)
@@ -890,11 +892,12 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
addr->len = addr_len;
addr->hash = hash ^ sk->sk_type;
atomic_set(&addr->refcnt, 1);
-
+
if (sun_path[0]) {
- struct path path;
- umode_t mode = S_IFSOCK |
+ mode = S_IFSOCK |
(SOCK_INODE(sock)->i_mode & ~current_umask());
+
+ memset(&path, 0, sizeof(path));
err = unix_mknod(sun_path, mode, &path);
if (err) {
if (err == -EEXIST)
--
1.7.5.4
^ permalink raw reply related
* Re: Regarding bottlenecks for high speed packet generation
From: Ajith Adapa @ 2012-10-26 2:49 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev
In-Reply-To: <CAD6jFUScXX0x7yU2f-Cq4B6RZdnQx4dkbADHBL9aYhzQmeKGgA@mail.gmail.com>
Hi,
> If the driver uses NAPI [1] / the chip supports that, then not. You
> can observe the hardware / software interrupts via ifpps or watching
> procfs or some other tools, or simply look at the driver
> implementation.
>
> [1] http://www.linuxfoundation.org/collaborate/workgroups/networking/napi
But does NAPI comes into picture in case of transmission of packets ?
I read it comes into picture
only in the case of reception of packets.
Regards,
Ajith
^ permalink raw reply
* Re: [PATCH 1/1] af_unix.c: fix the compile warning in unix_bind
From: David Miller @ 2012-10-26 3:16 UTC (permalink / raw)
To: windsdaemon; +Cc: edumazet, viro, xemul, netdev
In-Reply-To: <1351219727-6085-1-git-send-email-windsdaemon@gmail.com>
From: Jing Wang <windsdaemon@gmail.com>
Date: Fri, 26 Oct 2012 10:48:47 +0800
>
> Signed-off-by: Jing Wang <windsdaemon@gmail.com>
What compile warning? You need to describe what problem
you are solving, rather than assume we can figure it out
by reading your patch.
> -
> +
This doesn't do anything and should be in your patch.
^ permalink raw reply
* Re: Regarding bottlenecks for high speed packet generation
From: David Miller @ 2012-10-26 3:18 UTC (permalink / raw)
To: adapa.ajith; +Cc: danborkmann, netdev
In-Reply-To: <CADAe=+L2hXpYwH-XyNS2HyHQrMQRCmV_v-bs0Qp+sR-mVkzV4A@mail.gmail.com>
From: Ajith Adapa <adapa.ajith@gmail.com>
Date: Fri, 26 Oct 2012 08:19:15 +0530
> Hi,
>
>> If the driver uses NAPI [1] / the chip supports that, then not. You
>> can observe the hardware / software interrupts via ifpps or watching
>> procfs or some other tools, or simply look at the driver
>> implementation.
>>
>> [1] http://www.linuxfoundation.org/collaborate/workgroups/networking/napi
>
> But does NAPI comes into picture in case of transmission of packets ?
Reclaim of TX descriptors occurs via NAPI.
^ permalink raw reply
* Re: [PATCH 1/1] af_unix.c: fix the compile warning in unix_bind
From: jing wang @ 2012-10-26 3:24 UTC (permalink / raw)
To: David Miller; +Cc: edumazet, viro, xemul, netdev
In-Reply-To: <20121025.231641.1536209335816707842.davem@davemloft.net>
Thanks for your review and i'm sorry about this, the warning is :
CC net/unix/af_unix.o
net/unix/af_unix.c: In function 'unix_bind':
net/unix/af_unix.c:895: warning: 'path.dentry' may be used uninitialized
in this function
net/unix/af_unix.c:895: warning: 'path.mnt' may be used uninitialized in
this function
于 2012年10月26日 11:16, David Miller 写道:
> From: Jing Wang <windsdaemon@gmail.com>
> Date: Fri, 26 Oct 2012 10:48:47 +0800
>
>> Signed-off-by: Jing Wang <windsdaemon@gmail.com>
> What compile warning? You need to describe what problem
> you are solving, rather than assume we can figure it out
> by reading your patch.
>
>> -
>> +
> This doesn't do anything and should be in your patch.
^ permalink raw reply
* Re: [PATCH net-next 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Neal Cardwell @ 2012-10-26 3:31 UTC (permalink / raw)
To: Vijay Subramanian
Cc: Netdev, David Miller, Venkat Venkatsubra, Elliot Hughes,
Eric Dumazet
In-Reply-To: <1351212356-11964-1-git-send-email-subramanian.vijay@gmail.com>
On Thu, Oct 25, 2012 at 8:45 PM, Vijay Subramanian
<subramanian.vijay@gmail.com> wrote:
> If server socket is slow to accept() connections, request_socks can represent
> connections for which the three-way handshake is already done. From client's
> point of view, the connection is in ESTABLISHED state but on server side, socket
> is not in accept_queue or ESTABLISHED state. When the syn-ack timer expires,
> because of the order in which tests are performed, server can retransmit the
> synack repeatedly. Following patch prevents the server from retransmitting the
> synack needlessly (and prevents client from replying with ack). This reduces
> traffic when server is slow to accept() connections.
>
> If the server socket has received the third ack during connection establishment,
> this is remembered in inet_rsk(req)->acked. The request_sock will expire in
> around 30 seconds and will be dropped if it does not move into accept_queue.
>
> With help from Eric Dumazet.
>
> Reported-by: Elliot Hughes <enh@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Tested-by: Neal Cardwell <ncardwell@google.com>
Nice. Verified the bug and the fix.
neal
^ permalink raw reply
* Re: [PATCH] sctp: Clean up type-punning in sctp_cmd_t union
From: Vlad Yasevich @ 2012-10-26 3:48 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller, linux-sctp
In-Reply-To: <20121025235843.GA13809@neilslaptop.think-freely.org>
On 10/25/2012 07:58 PM, Neil Horman wrote:
> On Thu, Oct 25, 2012 at 05:42:15PM -0400, Vlad Yasevich wrote:
>> On 10/25/2012 04:47 PM, Neil Horman wrote:
>>> Lots of points in the sctp_cmd_interpreter function treat the sctp_cmd_t arg as
>>> a void pointer, even though they are written as various other types. Theres no
>>> need for this as doing so just leads to possible type-punning issues that could
>>> cause crashes, and if we remain type-consistent we can actually just remove the
>>> void * member of the union entirely.
>>>
>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com
>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>> CC: "David S. Miller" <davem@davemloft.net>
>>> CC: linux-sctp@vger.kernel.org
>>> ---
>>> include/net/sctp/command.h | 7 ++++---
>>> include/net/sctp/ulpqueue.h | 2 +-
>>> net/sctp/sm_sideeffect.c | 45 ++++++++++++++++++++++-----------------------
>>> net/sctp/ulpqueue.c | 3 +--
>>> 4 files changed, 28 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
>>> index 712b3be..7f1b0f3 100644
>>> --- a/include/net/sctp/command.h
>>> +++ b/include/net/sctp/command.h
>>> @@ -131,7 +131,6 @@ typedef union {
>>> sctp_state_t state;
>>> sctp_event_timeout_t to;
>>> unsigned long zero;
>>> - void *ptr;
>>> struct sctp_chunk *chunk;
>>> struct sctp_association *asoc;
>>> struct sctp_transport *transport;
>>> @@ -154,9 +153,12 @@ typedef union {
>>> * which takes an __s32 and returns a sctp_arg_t containing the
>>> * __s32. So, after foo = SCTP_I32(arg), foo.i32 == arg.
>>> */
>>> +#define SCTP_NULL_BYTE 0xAA
>>> static inline sctp_arg_t SCTP_NULL(void)
>>> {
>>> - sctp_arg_t retval; retval.ptr = NULL; return retval;
>>> + sctp_arg_t retval;
>>> + memset(&retval, SCTP_NULL_BYTE, sizeof(sctp_arg_t));
>>> + return retval;
>>
>> What's this for? Can't we just use retval.zero?
>>
>> -vlad
>>
> My intent was to highlight any users of sctp_arg_t when SCTP_NULL was passed.
> My thinking was that the 0xAA byte patern would be a good indicator. Although,
> admittedly I didn't see the zero argument there. Looking at it though, the zero
> member of the union is effectively unused. Strictly speaking its used for
> initalization of sctp_arg_t, but its done somewhat poorly, since theres no
> guarantee that an unsigned long will be the largest member of that union. Doing
> the memset guarantees the whole instance is set to a predefined value.
>
> I could go either way with this, would you rather we just have SCTP_NULL return
> retval = { .zero = 0}; or would you rather remove the zero initialization from
> SCTP_[NO]FORCE, and SCTP_ARG_CONSTRUCTOR and do the memset. I think the memset
> reduces to a single 64 bit assignment as long as the union doesn't exceed that
> size anyway, and it ensures that you initalize the whole union's storage if it
> does in the future. And if we remove the initialization step (I don't see that
> its needed in the three macros above anyway), then we can remove the zero member
> as well.
>
You need the initialization step, otherwise things might fail (they did
on IA64 a while back). That's why the zero member was added. You can
go with memset if you want, but I was primarily wondering why the 0xAA
pattern was there.
-vlad
> Let me know what you want to do here, and I can respin this.
> Best
> Neil
>
^ permalink raw reply
* [PATCH] net: usb: Fix memory leak on Tx data path
From: Hemant Kumar @ 2012-10-26 4:17 UTC (permalink / raw)
To: oneukum
Cc: gregkh, netdev, linux-usb, linux-kernel, linux-arm-msm,
Hemant Kumar
Driver anchors the tx urbs and defers the urb submission if
a transmit request comes when the interface is suspended.
Anchoring urb increments the urb reference count. These
deferred urbs are later accessed by calling usb_get_from_anchor()
for submission during interface resume. usb_get_from_anchor()
unanchors the urb but urb reference count remains same.
This causes the urb reference count to remain non-zero
after usb_free_urb() gets called and urb never gets freed.
Hence call usb_put_urb() after anchoring the urb to properly
balance the reference count for these deferred urbs. Also,
unanchor these deferred urbs during disconnect, to free them
up.
Signed-off-by: Hemant Kumar <hemantk@codeaurora.org>
---
drivers/net/usb/usbnet.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 1867fe2..00b7598 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1168,6 +1168,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
usb_anchor_urb(urb, &dev->deferred);
/* no use to process more packets */
netif_stop_queue(net);
+ usb_put_urb(urb);
spin_unlock_irqrestore(&dev->txq.lock, flags);
netdev_dbg(dev->net, "Delaying transmission for resumption\n");
goto deferred;
@@ -1317,6 +1318,8 @@ void usbnet_disconnect (struct usb_interface *intf)
cancel_work_sync(&dev->kevent);
+ usb_scuttle_anchored_urbs(&dev->deferred);
+
if (dev->driver_info->unbind)
dev->driver_info->unbind (dev, intf);
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related
* Re: [PATCH net-next 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Eric Dumazet @ 2012-10-26 5:06 UTC (permalink / raw)
To: Vijay Subramanian
Cc: netdev, davem, Venkat Venkatsubra, Elliot Hughes, Eric Dumazet
In-Reply-To: <1351212356-11964-1-git-send-email-subramanian.vijay@gmail.com>
On Thu, 2012-10-25 at 17:45 -0700, Vijay Subramanian wrote:
> If server socket is slow to accept() connections, request_socks can represent
> connections for which the three-way handshake is already done. From client's
> point of view, the connection is in ESTABLISHED state but on server side, socket
> is not in accept_queue or ESTABLISHED state. When the syn-ack timer expires,
> because of the order in which tests are performed, server can retransmit the
> synack repeatedly. Following patch prevents the server from retransmitting the
> synack needlessly (and prevents client from replying with ack). This reduces
> traffic when server is slow to accept() connections.
>
> If the server socket has received the third ack during connection establishment,
> this is remembered in inet_rsk(req)->acked. The request_sock will expire in
> around 30 seconds and will be dropped if it does not move into accept_queue.
>
> With help from Eric Dumazet.
>
> Reported-by: Elliot Hughes <enh@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> ---
> Ignoring "WARNING: line over 80 characters" in the interest of readability.
>
> Eric,
> What about your earlier patch that modified sk_acceptq_is_full()?
Acked-by: Eric Dumazet <edumazet@google.com>
Please note Elliott Hughes takes two t, and by the way didnt reported
this particular problem. but the fact that client got the SYNACK message
while socket was not yet available to the server side.
About earlier patch, this would break :
tcp_abort_on_overflow - BOOLEAN
If listening service is too slow to accept new connections,
reset them. Default state is FALSE. It means that if overflow
occurred due to a burst, connection will recover. Enable this
option _only_ if you are really sure that listening daemon
cannot be tuned to accept connections faster. Enabling this
option can harm clients of your server.
And even if tcp_abort_on_overflow is TRUE, earlier patch would break
some ability to have a momentary burst of SYN packets above the listen()
backlog.
^ permalink raw reply
* Re: [PATCH net-next 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Eric Dumazet @ 2012-10-26 5:32 UTC (permalink / raw)
To: Vijay Subramanian
Cc: netdev, davem, Venkat Venkatsubra, Elliot Hughes, Eric Dumazet
In-Reply-To: <1351227971.6537.278.camel@edumazet-glaptop>
On Fri, 2012-10-26 at 07:06 +0200, Eric Dumazet wrote:
> About earlier patch, this would break :
>
> tcp_abort_on_overflow - BOOLEAN
...
> And even if tcp_abort_on_overflow is TRUE, earlier patch would break
> some ability to have a momentary burst of SYN packets above the listen()
> backlog.
So instead of sending SYNACK immediately, we could try to defer it a
bit.
But before adding yet another trick, we should evaluate if this is
really needed.
The timer wheel we currently use on the listener socket limits us
because of the granularity of the timer (one single timer for all
requests)
^ permalink raw reply
* Re: Regarding bottlenecks for high speed packet generation
From: Ajith Adapa @ 2012-10-26 6:12 UTC (permalink / raw)
To: David Miller; +Cc: danborkmann, netdev
In-Reply-To: <20121025.231814.962022530355865788.davem@davemloft.net>
> >
> > But does NAPI comes into picture in case of transmission of packets ?
>
> Reclaim of TX descriptors occurs via NAPI.
You mean for every packet sent by the NIC card it drops a interrupt to
reclaim the TX buffers used ?
I am not able to understand why does NIC has to interrupt back instead
of freeing the memory in background.
Might be it does add some bottleneck in my case of transmitting traffic bursts.
Regards,
Ajith
^ permalink raw reply
* Re: [PATCH v3 net-next] r8169: enable ALDPS for power saving
From: David Miller @ 2012-10-26 6:15 UTC (permalink / raw)
To: romieu; +Cc: hayeswang, netdev, linux-kernel, jean
In-Reply-To: <20121024212003.GA11152@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed, 24 Oct 2012 23:20:03 +0200
> Hayes Wang <hayeswang@realtek.com> :
>> Enable ALDPS function to save power when link down. Note that the
>> feature should be set after the other PHY settings. And the firmware
>> is necessary. Don't enable it without loading the firmware.
>>
>> None of the firmware-free chipsets support ALDPS. Neither do the
>> RTL8168d/8111d.
>>
>> For 8136 series, make sure the ALDPS is disabled before loading the
>> firmware. For 8168 series, the ALDPS would be disabled automatically
>> when loading firmware. You must not disable it directly.
>>
>> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
>
> Acked-by: Francois Romieu <romieu@fr.zoreil.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] packet: minor: remove unused err assignment
From: David Miller @ 2012-10-26 6:17 UTC (permalink / raw)
To: dxchgb; +Cc: netdev
In-Reply-To: <20121024132723.GA21528@thinkbox>
From: Daniel Borkmann <dxchgb@gmail.com>
Date: Wed, 24 Oct 2012 15:27:24 +0200
> This tiny patch removes two unused err assignments. In those two cases the
> err variable is either overwritten with another value at a later point in
> time without having read the previous assigment, or it is assigned and the
> function returns without using/reading err after the assignment.
>
> Signed-off-by: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>
Applied, thanks.
^ permalink raw reply
* Re: [RFC PATCH net-next v2 2/5] rtnl/ipv6: use netconf msg to advertise forwarding status
From: David Miller @ 2012-10-26 6:18 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev
In-Reply-To: <1351094579-3911-3-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed, 24 Oct 2012 18:02:56 +0200
> +static inline int inet6_netconf_msgsize_devconf(int type)
Outside of header files, we don't add explicit inlines, we let
the compiler decide.
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] Allow to monitor protocol configuration
From: David Miller @ 2012-10-26 6:19 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev
In-Reply-To: <1351094579-3911-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed, 24 Oct 2012 18:02:54 +0200
> This patch serie aims to advertising via netlink any change in the network
> configuration of a protocol (network or transport). For example the announcement
> when ip forwarding parameter is toggled.
>
> It adds a new kind of messages, RTM_NEWNETCONF/RTM_GETNETCONF.
>
> The patch which adds the support of netconf messages in iproute2 will be sent
> when the feature will be included. If someone is interested to get it before,
> just tell me!
>
> v2: rename functions to be consistent
> specify type of the attribute to notify when calling
> inet[6]_netconf_notify_devconf(), goal is to notify only thinks that
> changed.
> add RTM_GETNETCONF support
>
> Comments are welcome.
Besides the inline function thing, this set looks fine to me.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox