* Re: [PATCH net-next v2 1/5] net: Introduce NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-07 22:08 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CAKgT0Ue9h8Pdb4OkBiWdL5bSNHPUkbZfBosrtZ4gQOAnuGiwRQ@mail.gmail.com>
On Thu, Dec 7, 2017 at 1:35 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Thu, Dec 7, 2017 at 10:44 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>> On Thu, Dec 7, 2017 at 10:13 AM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Thu, Dec 7, 2017 at 12:03 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>>>> @@ -7405,6 +7405,23 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
>>>> features &= ~dev->gso_partial_features;
>>>> }
>>>>
>>>> + if (features & NETIF_F_GRO_HW) {
>>>> + /* Hardware GRO depends on GRO and RXCSUM. */
>>>> + if (!(features & NETIF_F_GRO)) {
>>>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no GRO feature.\n");
>>>> + features &= ~NETIF_F_GRO_HW;
>>>> + }
>>>
>>> I'm not sure I agree with this part. The hardware offload should not
>>> be dependent on the software offload.
>>
>> As I said before, GRO_HW is basically hardware accelerated GRO. To me,
>> it makes perfect sense that GRO_HW depends on GRO.
>>
>>> If you are concerned with the
>>> XDP case and such you should probably just look at handling it more
>>> like how TSO is handled with a group of flags that aggregate GRO, LRO,
>>> and GRO_HW into a group of features that must be disabled to support
>>> XDP.
>>>
>>>> + if (!(features & NETIF_F_RXCSUM)) {
>>>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no RXCSUM feature.\n");
>>>> + features &= ~NETIF_F_GRO_HW;
>>>> + }
>>>> + /* Hardware GRO and LRO are mutually exclusive. */
>>>> + if (features & NETIF_F_LRO) {
>>>> + netdev_dbg(dev, "Dropping NETIF_F_LRO since GRO_HW is set.\n");
>>>> + features &= ~NETIF_F_LRO;
>>>> + }
>>>
>>> This is going to be problematic. What if you bond one interface that
>>> has LRO and one that has GRO_HW? It seems like this is going to cause
>>> the bond interface to lie and say LRO isn't there when it actually is,
>>> or worse yet it will force features off when they shouldn't be.
>>
>> This is just dropping incompatible features similar to how other
>> incompatible feature flags are dropped in netdev_fix_features().
>> Hardware cannot aggregate in both LRO and GRO_HW modes at the same
>> time. It's one or the other.
>
> I'm not saying this is on a single device. The problem is in the case
> of bonding you have combined 2 devices. We don't want to have an upper
> device saying LRO isn't there when it is. A bonding setup is supposed
> to advertise LRO as being enabled if a lower device has it present.
> You are now making this an either/or thing at the driver level
> generically when that isn't the case due to things like bridging and
> bonding. This is why I was arguing that it would be easier to just
> make this a super-set of LRO with one form that is reversible and one
> that isn't. As is you are introducing all sorts of regressions that
> are going to be problematic in any sort of mixed environment.
Are you saying that if LRO is dropped on a upper device (by the new
code added in this patch), all lower devices will need to drop it too?
If that's what you are saying, it's already taken care of. There is
netdev_sync_upper_features() and netdev_sync_lower_features() that
will automatically do that as part of __netdev_update_features().
>
>> LRO and GRO_HW are different. We may never agree on this but they are
>> different. If a bond needs to disable LRO, it will be propagated to
>> all slaves. But each slave can have GRO enabled. If GRO is enabled,
>> GRO_HW, being a subset of GRO, can be enabled.
>
> I get that they are very different. At a driver level it makes sense
> to say you get one or the other at a physical driver level. The point
> I was trying to make is that the bond is only able to set one or the
> other when you might have a mix of devices. On the bonding device you
> should be able to have both LRO and GRO_HW enabled.
On the bond, you can have LRO enabled and it is propagated to lower
devices so that lower devices will enable LRO if it is supported. If
LRO is disabled on the bond (e.g. due to bridging), It will be
propagated so all lower devices will disable LRO if it is enabled.
GRO/GRO_HW is different and it is not propagated from upper devices to
lower devices like LRO. Lower devices can have GRO/GRO_HW
enabled/disabled regardless of upper device. This is no different
from say RXCSUM. Lower devices can have RXCSUM on/off regardless.
^ permalink raw reply
* Re: [PATCH v2 net-next 3/4] bpftool: implement prog load command
From: Jakub Kicinski @ 2017-12-07 21:55 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev, linux-kernel, kernel-team, ast, daniel, kafai,
Quentin Monnet, David Ahern
In-Reply-To: <20171207183909.16240-4-guro@fb.com>
On Thu, 7 Dec 2017 18:39:08 +0000, Roman Gushchin wrote:
> Add the prog load command to load a bpf program from a specified
> binary file and pin it to bpffs.
>
> Usage description and examples are given in the corresponding man
> page.
>
> Syntax:
> $ bpftool prog load SOURCE_FILE FILE
>
> FILE is a non-existing file on bpffs.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
The only reservation I have is that SRC may confuse people, would it
make sense to call the compiled code OBJ? I'm afraid someone may try
to load a C source...
^ permalink raw reply
* Re: [Intel-wired-lan] [next-queue 06/10] ixgbe: restore offloaded SAs after a reset
From: Alexander Duyck @ 2017-12-07 21:52 UTC (permalink / raw)
To: Shannon Nelson
Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Sowmini Varadhan,
Netdev
In-Reply-To: <f8f1d753-3f29-6aa7-326c-423f38036877@oracle.com>
On Thu, Dec 7, 2017 at 10:47 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> On 12/7/2017 9:16 AM, Alexander Duyck wrote:
>>
>> On Wed, Dec 6, 2017 at 9:43 PM, Shannon Nelson
>> <shannon.nelson@oracle.com> wrote:
>>>
>>> On 12/5/2017 9:30 AM, Alexander Duyck wrote:
>>>>
>>>>
>>>> On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson
>>>> <shannon.nelson@oracle.com> wrote:
>>>>>
>>>>>
>>>>> On a chip reset most of the table contents are lost, so must be
>
>
> <snip>
>
>>>>> + /* reload the IP addrs */
>>>>> + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
>>>>> + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i];
>>>>> +
>>>>> + if (ipsa->used)
>>>>> + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr);
>>>>> + else
>>>>> + ixgbe_ipsec_set_rx_ip(hw, i, zbuf);
>>>>
>>>>
>>>>
>>>> If we are doing a restore do we actually need to write the zero
>>>> values? If we did a reset I thought you had a function that was going
>>>> through and zeroing everything out. If so this now becomes redundant.
>>>
>>>
>>>
>>> Currently ixgbe_ipsec_clear_hw_tables() only gets run at at probe. It
>>> should probably get run at remove as well. Doing this is a bit of safety
>>> paranoia, and making sure the CAM memory structures that don't get
>>> cleared
>>> on reset have exactly what I expect in them.
>>
>>
>> You might just move ixgbe_ipsec_clear_hw_tables into the rest logic
>> itself. Then it covers all cases where you would be resetting the
>> hardware and expecting a consistent state. It will mean writing some
>> registers twice during the reset but it is probably better just to
>> make certain everything stays in a known good state after a reset.
>
>
> If it is a small number, e.g. 10 or 20, then you may be right. However,
> given we have table space for 2k different SAs, at 6 writes per Tx SA and 10
> writes per Rx SA, plus 128 IP address with 4 writes each, we are already
> looking at 17K writes already to be sure the tables are clean.
>
> Unfortunately, I don't really know what a "typical" case will be, so I don't
> know how many SA we may be offloading at any one time. But in a busy cloud
> support server, we might have nearly full tables. If we do the full clean
> first, then have to fill all the tables, we're now looking at up to 35k
> writes slowing down the reset process.
>
> I'd rather keep it to the constant 17K writes for now, and look later at
> using the VALID bit in the IPSRXMOD to see if we can at least cut down on
> the Rx writes.
>
> sln
The reads/writes themselves should be cheap. These kind of things only
get to be really expensive when you start looking at adding delays in
between the writes/reads polling on things. As long as we aren't
waiting milliseconds on things you can write/read thousands of
registers and not even notice it.
One thing you might look at doing in order to speed some of this up a
bit would be to also combine updating the Tx SA and Rx SA in your
clear_hw_tables loop so that you could do them in parallel in your
loop instead of having to do them in series. Anyway it is just a
thought. If nothing else you might look at timing the function to see
how long it actually takes. I suspect it shouldn't be too long since
the turnaround time on the PCIe bus should be in microseconds so odds
are reading/writing 35K registers might ovinly add a few milliseconds
to total reset time.
^ permalink raw reply
* Re: [PATCH 2/3] bluetooth: hci_ll: add constant for vendor-specific command
From: Marcel Holtmann @ 2017-12-07 21:43 UTC (permalink / raw)
To: David Lechner
Cc: devicetree, open list:BLUETOOTH DRIVERS, Rob Herring,
Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
Network Development, linux-kernel
In-Reply-To: <55fff19a-c30f-88f0-afb2-6c0b9bad25a7@lechnology.com>
Hi David,
>>> This adds a #define for the vendor-specific HCI command to set the
>>> baudrate instead of using the bare 0xff36 multiple times.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>> drivers/bluetooth/hci_ll.c | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>> patch has been applied to bluetooth-next tree.
>
> I am new to the bluetooth tree, so just to be sure... does this mean I should not include this patch in v2 of this series since this patch has been applied?
please don’t include already applied patches. Base everything against bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* [PATCH net] tcp: invalidate rate samples during SACK reneging
From: Yousuk Seung @ 2017-12-07 21:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Yousuk Seung, Neal Cardwell, Yuchung Cheng
Mark tcp_sock during a SACK reneging event and invalidate rate samples
while marked. Such rate samples may overestimate bw by including packets
that were SACKed before reneging.
< ack 6001 win 10000 sack 7001:38001
< ack 7001 win 0 sack 8001:38001 // Reneg detected
> seq 7001:8001 // RTO, SACK cleared.
< ack 38001 win 10000
In above example the rate sample taken after the last ack will count
7001-38001 as delivered while the actual delivery rate likely could
be much lower i.e. 7001-8001.
This patch adds a new field tcp_sock.sack_reneg and marks it when we
declare SACK reneging and entering TCP_CA_Loss, and unmarks it after
the last rate sample was taken before moving back to TCP_CA_Open. This
patch also invalidates rate samples taken while tcp_sock.is_sack_reneg
is set.
Fixes: b9f64820fb22 ("tcp: track data delivery rate for a TCP connection")
Signed-off-by: Yousuk Seung <ysseung@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Priyaranjan Jha <priyarjha@google.com>
---
include/linux/tcp.h | 3 ++-
include/net/tcp.h | 2 +-
net/ipv4/tcp.c | 1 +
net/ipv4/tcp_input.c | 10 ++++++++--
net/ipv4/tcp_rate.c | 10 +++++++---
5 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index df5d97a85e1a..ca4a6361389b 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -224,7 +224,8 @@ struct tcp_sock {
rate_app_limited:1, /* rate_{delivered,interval_us} limited? */
fastopen_connect:1, /* FASTOPEN_CONNECT sockopt */
fastopen_no_cookie:1, /* Allow send/recv SYN+data without a cookie */
- unused:3;
+ is_sack_reneg:1, /* in recovery from loss with SACK reneg? */
+ unused:2;
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
unused1 : 1,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 6998707e81f3..6da880d2f022 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1055,7 +1055,7 @@ void tcp_rate_skb_sent(struct sock *sk, struct sk_buff *skb);
void tcp_rate_skb_delivered(struct sock *sk, struct sk_buff *skb,
struct rate_sample *rs);
void tcp_rate_gen(struct sock *sk, u32 delivered, u32 lost,
- struct rate_sample *rs);
+ bool is_sack_reneg, struct rate_sample *rs);
void tcp_rate_check_app_limited(struct sock *sk);
/* These functions determine how the current flow behaves in respect of SACK
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index bf97317e6c97..f08eebe60446 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2412,6 +2412,7 @@ int tcp_disconnect(struct sock *sk, int flags)
tp->snd_cwnd_cnt = 0;
tp->window_clamp = 0;
tcp_set_ca_state(sk, TCP_CA_Open);
+ tp->is_sack_reneg = 0;
tcp_clear_retrans(tp);
inet_csk_delack_init(sk);
/* Initialize rcv_mss to TCP_MIN_MSS to avoid division by 0
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 734cfc8ff76e..7b9e8b99b28d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1941,6 +1941,8 @@ void tcp_enter_loss(struct sock *sk)
if (is_reneg) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSACKRENEGING);
tp->sacked_out = 0;
+ /* Mark SACK reneging until we recover from this loss event. */
+ tp->is_sack_reneg = 1;
}
tcp_clear_all_retrans_hints(tp);
@@ -2364,6 +2366,7 @@ static bool tcp_try_undo_recovery(struct sock *sk)
return true;
}
tcp_set_ca_state(sk, TCP_CA_Open);
+ tp->is_sack_reneg = 0;
return false;
}
@@ -2397,8 +2400,10 @@ static bool tcp_try_undo_loss(struct sock *sk, bool frto_undo)
NET_INC_STATS(sock_net(sk),
LINUX_MIB_TCPSPURIOUSRTOS);
inet_csk(sk)->icsk_retransmits = 0;
- if (frto_undo || tcp_is_sack(tp))
+ if (frto_undo || tcp_is_sack(tp)) {
tcp_set_ca_state(sk, TCP_CA_Open);
+ tp->is_sack_reneg = 0;
+ }
return true;
}
return false;
@@ -3495,6 +3500,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
struct tcp_sacktag_state sack_state;
struct rate_sample rs = { .prior_delivered = 0 };
u32 prior_snd_una = tp->snd_una;
+ bool is_sack_reneg = tp->is_sack_reneg;
u32 ack_seq = TCP_SKB_CB(skb)->seq;
u32 ack = TCP_SKB_CB(skb)->ack_seq;
bool is_dupack = false;
@@ -3611,7 +3617,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
delivered = tp->delivered - delivered; /* freshly ACKed or SACKed */
lost = tp->lost - lost; /* freshly marked lost */
- tcp_rate_gen(sk, delivered, lost, sack_state.rate);
+ tcp_rate_gen(sk, delivered, lost, is_sack_reneg, sack_state.rate);
tcp_cong_control(sk, ack, delivered, flag, sack_state.rate);
tcp_xmit_recovery(sk, rexmit);
return 1;
diff --git a/net/ipv4/tcp_rate.c b/net/ipv4/tcp_rate.c
index 3330a370d306..c61240e43923 100644
--- a/net/ipv4/tcp_rate.c
+++ b/net/ipv4/tcp_rate.c
@@ -106,7 +106,7 @@ void tcp_rate_skb_delivered(struct sock *sk, struct sk_buff *skb,
/* Update the connection delivery information and generate a rate sample. */
void tcp_rate_gen(struct sock *sk, u32 delivered, u32 lost,
- struct rate_sample *rs)
+ bool is_sack_reneg, struct rate_sample *rs)
{
struct tcp_sock *tp = tcp_sk(sk);
u32 snd_us, ack_us;
@@ -124,8 +124,12 @@ void tcp_rate_gen(struct sock *sk, u32 delivered, u32 lost,
rs->acked_sacked = delivered; /* freshly ACKed or SACKed */
rs->losses = lost; /* freshly marked lost */
- /* Return an invalid sample if no timing information is available. */
- if (!rs->prior_mstamp) {
+ /* Return an invalid sample if no timing information is available or
+ * in recovery from loss with SACK reneging. Rate samples taken during
+ * a SACK reneging event may overestimate bw by including packets that
+ * were SACKed before the reneg.
+ */
+ if (!rs->prior_mstamp || is_sack_reneg) {
rs->delivered = -1;
rs->interval_us = -1;
return;
--
2.15.1.424.g9478a66081-goog
^ permalink raw reply related
* Re: [PATCH net-next v2 1/5] net: Introduce NETIF_F_GRO_HW.
From: Alexander Duyck @ 2017-12-07 21:35 UTC (permalink / raw)
To: Michael Chan
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CACKFLimmyQEDs8Ti3SqVD2Tq2uEjQv7En+BORgQSj8tqh0mx3A@mail.gmail.com>
On Thu, Dec 7, 2017 at 10:44 AM, Michael Chan <michael.chan@broadcom.com> wrote:
> On Thu, Dec 7, 2017 at 10:13 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Thu, Dec 7, 2017 at 12:03 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>>> @@ -7405,6 +7405,23 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
>>> features &= ~dev->gso_partial_features;
>>> }
>>>
>>> + if (features & NETIF_F_GRO_HW) {
>>> + /* Hardware GRO depends on GRO and RXCSUM. */
>>> + if (!(features & NETIF_F_GRO)) {
>>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no GRO feature.\n");
>>> + features &= ~NETIF_F_GRO_HW;
>>> + }
>>
>> I'm not sure I agree with this part. The hardware offload should not
>> be dependent on the software offload.
>
> As I said before, GRO_HW is basically hardware accelerated GRO. To me,
> it makes perfect sense that GRO_HW depends on GRO.
>
>> If you are concerned with the
>> XDP case and such you should probably just look at handling it more
>> like how TSO is handled with a group of flags that aggregate GRO, LRO,
>> and GRO_HW into a group of features that must be disabled to support
>> XDP.
>>
>>> + if (!(features & NETIF_F_RXCSUM)) {
>>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no RXCSUM feature.\n");
>>> + features &= ~NETIF_F_GRO_HW;
>>> + }
>>> + /* Hardware GRO and LRO are mutually exclusive. */
>>> + if (features & NETIF_F_LRO) {
>>> + netdev_dbg(dev, "Dropping NETIF_F_LRO since GRO_HW is set.\n");
>>> + features &= ~NETIF_F_LRO;
>>> + }
>>
>> This is going to be problematic. What if you bond one interface that
>> has LRO and one that has GRO_HW? It seems like this is going to cause
>> the bond interface to lie and say LRO isn't there when it actually is,
>> or worse yet it will force features off when they shouldn't be.
>
> This is just dropping incompatible features similar to how other
> incompatible feature flags are dropped in netdev_fix_features().
> Hardware cannot aggregate in both LRO and GRO_HW modes at the same
> time. It's one or the other.
I'm not saying this is on a single device. The problem is in the case
of bonding you have combined 2 devices. We don't want to have an upper
device saying LRO isn't there when it is. A bonding setup is supposed
to advertise LRO as being enabled if a lower device has it present.
You are now making this an either/or thing at the driver level
generically when that isn't the case due to things like bridging and
bonding. This is why I was arguing that it would be easier to just
make this a super-set of LRO with one form that is reversible and one
that isn't. As is you are introducing all sorts of regressions that
are going to be problematic in any sort of mixed environment.
> LRO and GRO_HW are different. We may never agree on this but they are
> different. If a bond needs to disable LRO, it will be propagated to
> all slaves. But each slave can have GRO enabled. If GRO is enabled,
> GRO_HW, being a subset of GRO, can be enabled.
I get that they are very different. At a driver level it makes sense
to say you get one or the other at a physical driver level. The point
I was trying to make is that the bond is only able to set one or the
other when you might have a mix of devices. On the bonding device you
should be able to have both LRO and GRO_HW enabled.
^ permalink raw reply
* Re: [PATCH v5 2/2] sock: Move the socket inuse to namespace.
From: Cong Wang @ 2017-12-07 21:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tonghao Zhang, David Miller, Eric Dumazet, Willem de Bruijn,
Linux Kernel Network Developers
In-Reply-To: <1512667208.25033.13.camel@gmail.com>
On Thu, Dec 7, 2017 at 9:20 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2017-12-07 at 08:45 -0800, Tonghao Zhang wrote:
>> In some case, we want to know how many sockets are in use in
>> different _net_ namespaces. It's a key resource metric.
>>
>
> ...
>
>> +static void sock_inuse_add(struct net *net, int val)
>> +{
>> + if (net->core.prot_inuse)
>> + this_cpu_add(*net->core.sock_inuse, val);
>> +}
>
> This is very confusing.
>
> Why testing net->core.prot_inuse for NULL is needed at all ?
>
> Why not testing net->core.sock_inuse instead ?
I bet that is copy-n-paste error given that sock_inuse_exit_net()
has a similar typo.
^ permalink raw reply
* Re: [PATCH 2/3] bluetooth: hci_ll: add constant for vendor-specific command
From: David Lechner @ 2017-12-07 21:29 UTC (permalink / raw)
To: Marcel Holtmann
Cc: devicetree, open list:BLUETOOTH DRIVERS, Rob Herring,
Mark Rutland, Gustavo F. Padovan, Johan Hedberg, netdev,
linux-kernel
In-Reply-To: <5C65FB74-D7FD-4653-993A-5A68AD844A41@holtmann.org>
On 12/04/2017 12:23 AM, Marcel Holtmann wrote:
> Hi David,
>
>> This adds a #define for the vendor-specific HCI command to set the
>> baudrate instead of using the bare 0xff36 multiple times.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>> drivers/bluetooth/hci_ll.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> patch has been applied to bluetooth-next tree.
>
I am new to the bluetooth tree, so just to be sure... does this mean I
should not include this patch in v2 of this series since this patch has
been applied?
^ permalink raw reply
* Re: [PATCH net-next] netdevsim: check return value of debugfs_create_dir
From: Jakub Kicinski @ 2017-12-07 21:20 UTC (permalink / raw)
To: Prashant Bhole; +Cc: 'David S . Miller', netdev
In-Reply-To: <001001d36f39$4a81b5b0$df852110$@lab.ntt.co.jp>
On Thu, 7 Dec 2017 17:56:37 +0900, Prashant Bhole wrote:
> > From: Jakub Kicinski [mailto:jakub.kicinski@netronome.com]
> >
> > On Thu, 7 Dec 2017 13:10:39 +0900, Prashant Bhole wrote:
> > > > From: Jakub Kicinski [mailto:jakub.kicinski@netronome.com]
> > > >
> > > > On Thu, 7 Dec 2017 10:02:13 +0900, Prashant Bhole wrote:
> > > > > - Handled debugfs_create_dir failure in nsim_init()
> > > > > - Fixed return value of nsim_module_init() when debugfs_create_dir
> > > > > fails
> > > > >
> > > > > Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> > > >
> > > > Why? Failing to expose the state via DebugFS is not fatal to the driver.
> > >
> > > Ok, my intention was to handle the return code properly, which is not
> > > needed as per your comment.
> > > Shall I remove the existing handling in nsim_module_init() in separate
> > > patch?
> >
> > I was going back and forth on the error handling quite a bit writing that code. In
> > the end I decided to leave the module_init check and check for bpf prog
> > directory. Former one is mostly useful to make sure the is no duplicate directory
> > with the same name, the latter to limit possible false positive in the selftest..
>
> Ok. Currently return value is checked with IS_ERR(). But when Debug FS is
> enabled, debugfs_create_dir will never return error value. It returns either
> NULL or a valid pointer. Shall I replace IS_ERR with NULL check or
> IS_ERR_OR_NULL check?
Sure.
^ permalink raw reply
* Re: [PATCH net-next 1/1] net: dsa: microchip: Add Microchip KSZ8895 DSA driver
From: Pavel Machek @ 2017-12-07 21:04 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, f.fainelli, muvarov, nathan.leigh.conrad, vivien.didelot,
UNGLinuxDriver, netdev
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD4113DCE0@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 5138 bytes --]
Hi!
> > I went back to my version of dsa patches, and test above works as
> > expected.
>
> Sorry to be this late for the reply. I finally got hold of a KSZ8895 board that
> works with my SoC board to confirm the network communication.
>
> As expected the KSZ8895 board works correctly as the chip uses the same
> tail tagging feature in KSZ8795, and I did verify that board is
> working.
Ok, let me retry:
> One thing to debug this problem is to dump the MIB counters. Use the ethtool
> utility to show MIB counters of both ports:
>
> ethtool -S lan3
> ethtool -S eth0
>
> Assuming eth0 is the MAC controller that drives the switch, the receive counters of
> the host port of the switch should match the transmit counters of
> lan3, and vice versa.
Hmm. I'm getting some "interesting" results from mii-tool:
root@miro:~# mii-tool lan3
lan3: negotiated 1000baseT-HD flow-control, link ok
But IIRC the switch is 100mbit? And dmesg does get it right. Its just
mii-tool that is confused.
Link detection seems to work
root@miro:/sys/bus/spi/devices/spi2.0# mii-tool lan1
lan1: negotiated 1000baseT-HD flow-control, link ok
root@miro:/sys/bus/spi/devices/spi2.0# mii-tool lan1
lan1: no link
(But that really should be 100baseT, not 1000baseT).
Is there register dump available somewhere? I was using
/sys/bus/spi/devices/spi32766.0/registers but this does not seem to be
available.
I tried that ethtool -S, and counters do not seem to match:
Help welcome.
Best regards,
Pavel
root@miro:~# ifconfig eth0 up
root@miro:~# ifconfig lan3 192.168.20.103 netmask 255.255.0.0 up
[ 180.903675] IPv6: ADDRCONF(NETDEV_UP): lan3: link is not ready
root@miro:~# [ 181.913284] ksz8895-switch spi2.0 lan3: Link is Up - 100Mbps/Full - flow control rx/tx
[ 181.921358] IPv6: ADDRCONF(NETDEV_CHANGE): lan3: link becomes ready
root@miro:~# route add default gw 192.168.1.1
root@miro:~# ping 192.168.1.1
PING 192.168.1.1 (192.168.1.1): 56 data bytes
^C
--- 192.168.1.1 ping statistics ---
17 packets transmitted, 0 packets received, 100% packet loss
root@miro:~# ethtool -S eth0
NIC statistics:
tx_dropped: 0
tx_packets: 55
tx_broadcast: 35
tx_multicast: 20
tx_crc_errors: 0
tx_undersize: 0
tx_oversize: 0
tx_fragment: 0
tx_jabber: 0
tx_collision: 0
tx_64byte: 0
tx_65to127byte: 35
tx_128to255byte: 0
tx_256to511byte: 20
tx_512to1023byte: 0
tx_1024to2047byte: 0
tx_GTE2048byte: 0
tx_octets: 9576
IEEE_tx_drop: 0
IEEE_tx_frame_ok: 55
IEEE_tx_1col: 0
IEEE_tx_mcol: 0
IEEE_tx_def: 0
IEEE_tx_lcol: 0
IEEE_tx_excol: 0
IEEE_tx_macerr: 0
IEEE_tx_cserr: 0
IEEE_tx_sqe: 0
IEEE_tx_fdxfc: 0
IEEE_tx_octets_ok: 9576
rx_packets: 0
rx_broadcast: 0
rx_multicast: 0
rx_crc_errors: 0
rx_undersize: 0
rx_oversize: 0
rx_fragment: 0
rx_jabber: 0
rx_64byte: 0
rx_65to127byte: 0
rx_128to255byte: 0
rx_256to511byte: 0
rx_512to1023byte: 0
rx_1024to2047byte: 0
rx_GTE2048byte: 0
rx_octets: 0
IEEE_rx_drop: 0
IEEE_rx_frame_ok: 0
IEEE_rx_crc: 0
IEEE_rx_align: 0
IEEE_rx_macerr: 0
IEEE_rx_fdxfc: 0
IEEE_rx_octets_ok: 0
p04_rx: 660
p04_rx_hi: 0
p04_rx_undersize: 0
p04_rx_fragments: 20
p04_rx_oversize: 0
p04_rx_jabbers: 0
p04_rx_symbol_err: 0
p04_rx_crc_err: 0
p04_rx_align_err: 0
p04_rx_mac_ctrl: 0
p04_rx_pause: 0
p04_rx_bcast: 0
p04_rx_mcast: 0
p04_rx_ucast: 0
p04_rx_64_or_less: 0
p04_rx_65_127: 0
p04_rx_128_255: 0
p04_rx_256_511: 0
p04_rx_512_1023: 0
p04_rx_1024_1522: 0
p04_tx: 388
p04_tx_hi: 0
p04_tx_late_col: 0
p04_tx_pause: 0
p04_tx_bcast: 0
p04_tx_mcast: 3
p04_tx_ucast: 0
p04_tx_deferred: 0
p04_tx_total_col: 0
p04_tx_exc_col: 0
p04_tx_single_col: 0
p04_tx_mult_col: 0
p04_rx_discards: 0
p04_tx_discards: 0
root@miro:~# ethtool -S lan3
NIC statistics:
tx_packets: 24
tx_bytes: 1356
rx_packets: 0
rx_bytes: 0
rx: 566
rx_hi: 0
rx_undersize: 0
rx_fragments: 0
rx_oversize: 0
rx_jabbers: 0
rx_symbol_err: 0
rx_crc_err: 0
rx_align_err: 0
rx_mac_ctrl: 0
rx_pause: 0
rx_bcast: 0
rx_mcast: 4
rx_ucast: 0
rx_64_or_less: 0
rx_65_127: 1
rx_128_255: 3
rx_256_511: 0
rx_512_1023: 0
rx_1024_1522: 0
tx: 0
tx_hi: 0
tx_late_col: 0
tx_pause: 0
tx_bcast: 0
tx_mcast: 0
tx_ucast: 0
tx_deferred: 0
tx_total_col: 0
tx_exc_col: 0
tx_single_col: 0
tx_mult_col: 0
rx_discards: 0
tx_discards: 0
root@miro:~#
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: lan9303: Protect ALR operations with mutex
From: Andrew Lunn @ 2017-12-07 20:58 UTC (permalink / raw)
To: Egil Hjelmeland; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel
In-Reply-To: <20171207185604.30745-1-privat@egil-hjelmeland.no>
On Thu, Dec 07, 2017 at 07:56:04PM +0100, Egil Hjelmeland wrote:
> ALR table operations are a sequence of related register operations which
> should be protected from concurrent access. The alr_cache should also be
> protected. Add alr_mutex doing that.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH] xfrm: fix xfrm_do_migrate() with AEAD e.g(AES-GCM)
From: Antony Antony @ 2017-12-07 20:54 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu; +Cc: netdev, David S . Miller, Antony Antony
copy geniv when cloning the xfrm state.
x->geniv was not copied to the new state and migration would fail.
xfrm_do_migrate
..
xfrm_state_clone()
..
..
esp_init_aead()
crypto_alloc_aead()
crypto_alloc_tfm()
crypto_find_alg() return EAGAIN and failed
Signed-off-by: Antony Antony <antony@phenome.org>
---
net/xfrm/xfrm_state.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 065d89606888..500b3391f474 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1343,6 +1343,7 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
if (orig->aead) {
x->aead = xfrm_algo_aead_clone(orig->aead);
+ x->geniv = orig->geniv;
if (!x->aead)
goto error;
}
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next] net: ethernet: ti: cpdma: rate is not changed - correct case
From: Ivan Khoronzhuk @ 2017-12-07 20:21 UTC (permalink / raw)
To: David Miller; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171207.151315.1535972470633975595.davem@davemloft.net>
On Thu, Dec 07, 2017 at 03:13:15PM -0500, David Miller wrote:
> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Date: Thu, 7 Dec 2017 22:10:06 +0200
>
> > On Thu, Dec 07, 2017 at 02:50:24PM -0500, David Miller wrote:
> >> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> Date: Thu, 7 Dec 2017 21:48:56 +0200
> >>
> >> > On Wed, Dec 06, 2017 at 04:35:45PM -0500, David Miller wrote:
> >> >> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> >> Date: Wed, 6 Dec 2017 16:41:18 +0200
> >> >>
> >> >> > If rate is the same as set it's correct case.
> >> >> >
> >> >> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> >> > ---
> >> >> > Based on net-next/master
> >> >> >
> >> >> > drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
> >> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> >> >> > index e4d6edf..dbe9167 100644
> >> >> > --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> >> >> > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> >> >> > @@ -841,7 +841,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
> >> >> > return -EINVAL;
> >> >> >
> >> >> > if (ch->rate == rate)
> >> >> > - return rate;
> >> >> > + return 0;
> >> >>
> >> >> Looking at the one and only caller of this function, cpsw_ndo_set_tx_maxrate, it
> >> >> makes sure this can never, ever, happen.
> >> > In current circumstances yes, it will never happen.
> >> > But I caught it while adding related code and better return 0 if upper caller
> >> > doesn't have such check. Suppose that cpdma module is responsible for itself
> >> > and if it's critical I can send this patch along with whole related series.
> >>
> >> You have to decide one way or the other, who is responsible.
> >>
> >> I think checking higher up is better because it's cheaper at that point to
> >> look at the per-netdev queue rate setting before moving down deeper into the
> >> driver specific data-structures.
> >
> > No objection, but upper caller not always knows current rate and for doing like
> > this it needs read it first, and this is also some redundancy.
>
> How can the upper caller not know the current rate? The rate is
> always stored in the generic netdev per-queue datastructure.
>
> And that's what existing code checks right now.
Right now, when generic netdev only caller - yes.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: rate is not changed - correct case
From: David Miller @ 2017-12-07 20:13 UTC (permalink / raw)
To: ivan.khoronzhuk; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171207201005.GC3022@khorivan>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Thu, 7 Dec 2017 22:10:06 +0200
> On Thu, Dec 07, 2017 at 02:50:24PM -0500, David Miller wrote:
>> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> Date: Thu, 7 Dec 2017 21:48:56 +0200
>>
>> > On Wed, Dec 06, 2017 at 04:35:45PM -0500, David Miller wrote:
>> >> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >> Date: Wed, 6 Dec 2017 16:41:18 +0200
>> >>
>> >> > If rate is the same as set it's correct case.
>> >> >
>> >> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >> > ---
>> >> > Based on net-next/master
>> >> >
>> >> > drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
>> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
>> >> > index e4d6edf..dbe9167 100644
>> >> > --- a/drivers/net/ethernet/ti/davinci_cpdma.c
>> >> > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
>> >> > @@ -841,7 +841,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
>> >> > return -EINVAL;
>> >> >
>> >> > if (ch->rate == rate)
>> >> > - return rate;
>> >> > + return 0;
>> >>
>> >> Looking at the one and only caller of this function, cpsw_ndo_set_tx_maxrate, it
>> >> makes sure this can never, ever, happen.
>> > In current circumstances yes, it will never happen.
>> > But I caught it while adding related code and better return 0 if upper caller
>> > doesn't have such check. Suppose that cpdma module is responsible for itself
>> > and if it's critical I can send this patch along with whole related series.
>>
>> You have to decide one way or the other, who is responsible.
>>
>> I think checking higher up is better because it's cheaper at that point to
>> look at the per-netdev queue rate setting before moving down deeper into the
>> driver specific data-structures.
>
> No objection, but upper caller not always knows current rate and for doing like
> this it needs read it first, and this is also some redundancy.
How can the upper caller not know the current rate? The rate is
always stored in the generic netdev per-queue datastructure.
And that's what existing code checks right now.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: rate is not changed - correct case
From: Ivan Khoronzhuk @ 2017-12-07 20:10 UTC (permalink / raw)
To: David Miller; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171207.145024.1434883857028947517.davem@davemloft.net>
On Thu, Dec 07, 2017 at 02:50:24PM -0500, David Miller wrote:
> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Date: Thu, 7 Dec 2017 21:48:56 +0200
>
> > On Wed, Dec 06, 2017 at 04:35:45PM -0500, David Miller wrote:
> >> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> Date: Wed, 6 Dec 2017 16:41:18 +0200
> >>
> >> > If rate is the same as set it's correct case.
> >> >
> >> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> > ---
> >> > Based on net-next/master
> >> >
> >> > drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> >> > index e4d6edf..dbe9167 100644
> >> > --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> >> > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> >> > @@ -841,7 +841,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
> >> > return -EINVAL;
> >> >
> >> > if (ch->rate == rate)
> >> > - return rate;
> >> > + return 0;
> >>
> >> Looking at the one and only caller of this function, cpsw_ndo_set_tx_maxrate, it
> >> makes sure this can never, ever, happen.
> > In current circumstances yes, it will never happen.
> > But I caught it while adding related code and better return 0 if upper caller
> > doesn't have such check. Suppose that cpdma module is responsible for itself
> > and if it's critical I can send this patch along with whole related series.
>
> You have to decide one way or the other, who is responsible.
>
> I think checking higher up is better because it's cheaper at that point to
> look at the per-netdev queue rate setting before moving down deeper into the
> driver specific data-structures.
No objection, but upper caller not always knows current rate and for doing like
this it needs read it first, and this is also some redundancy.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net-next 0/6] smc: fixes 2017-12-07
From: David Miller @ 2017-12-07 20:06 UTC (permalink / raw)
To: ubraun; +Cc: netdev, linux-s390, jwi, schwidefsky, heiko.carstens, raspl
In-Reply-To: <20171207123849.52910-1-ubraun@linux.vnet.ibm.com>
From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Thu, 7 Dec 2017 13:38:43 +0100
> here are some smc-patches. The initial 4 patches are cleanups.
> Patch 5 gets rid of ib_post_sends in tasklet context to avoid peer drops due
> to out-of-order receivals.
> Patch 6 makes sure, the Linux SMC code understands variable sized CLC proposal
> messages built according to RFC7609.
Series applied to net-next.
^ permalink raw reply
* Re: [PATCH v5 net-next] net/tcp: trace all TCP/IP state transition with tcp_set_state tracepoint
From: David Miller @ 2017-12-07 20:04 UTC (permalink / raw)
To: marcelo.leitner
Cc: laoar.shao, songliubraving, kuznet, yoshfuji, rostedt, bgregg,
netdev, linux-kernel
In-Reply-To: <20171207200245.GQ13341@localhost.localdomain>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 7 Dec 2017 18:02:46 -0200
> Dave?
Yeah that will untangle this mess much more nicely.
^ permalink raw reply
* Re: [PATCH v5 net-next] net/tcp: trace all TCP/IP state transition with tcp_set_state tracepoint
From: Marcelo Ricardo Leitner @ 2017-12-07 20:02 UTC (permalink / raw)
To: Yafang Shao
Cc: davem, songliubraving, kuznet, yoshfuji, rostedt, bgregg, netdev,
linux-kernel
In-Reply-To: <1512655842-17784-1-git-send-email-laoar.shao@gmail.com>
On Thu, Dec 07, 2017 at 02:10:42PM +0000, Yafang Shao wrote:
> The TCP/IP transition from TCP_LISTEN to TCP_SYN_RECV and some other
> transitions are not traced with tcp_set_state tracepoint.
>
> In order to trace the whole tcp lifespans, two helpers are introduced,
> void sk_set_state(struct sock *sk, int state);
> void sk_state_store(struct sock *sk, int newstate);
>
> When do TCP/IP state transition, we should use these two helpers or use
> tcp_set_state() other than assigning a value to sk_state directly.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> v4->v5: Trace only TCP sockets, whatever it is stream socket or raw socket.
> v3->v4: Do not trace DCCP socket
> v2->v3: Per suggestion from Marcelo Ricardo Leitner, inverting __
> to sk_state_store.
> ---
> include/net/sock.h | 8 ++++++--
> net/core/sock.c | 15 +++++++++++++++
> net/ipv4/inet_connection_sock.c | 5 +++--
> net/ipv4/inet_hashtables.c | 2 +-
> net/ipv4/tcp.c | 2 +-
> 5 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 79e1a2c..1cf7685 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2349,18 +2349,22 @@ static inline int sk_state_load(const struct sock *sk)
> }
>
> /**
> - * sk_state_store - update sk->sk_state
> + * __sk_state_store - update sk->sk_state
> * @sk: socket pointer
> * @newstate: new state
> *
> * Paired with sk_state_load(). Should be used in contexts where
> * state change might impact lockless readers.
> */
> -static inline void sk_state_store(struct sock *sk, int newstate)
> +static inline void __sk_state_store(struct sock *sk, int newstate)
> {
> smp_store_release(&sk->sk_state, newstate);
> }
>
> +/* For tcp_set_state tracepoint */
> +void sk_state_store(struct sock *sk, int newstate);
> +void sk_set_state(struct sock *sk, int state);
> +
> void sock_enable_timestamp(struct sock *sk, int flag);
> int sock_get_timestamp(struct sock *, struct timeval __user *);
> int sock_get_timestampns(struct sock *, struct timespec __user *);
> diff --git a/net/core/sock.c b/net/core/sock.c
> index c0b5b2f..61841a2 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -138,6 +138,7 @@
> #include <net/sock_reuseport.h>
>
> #include <trace/events/sock.h>
> +#include <trace/events/tcp.h>
>
> #include <net/tcp.h>
> #include <net/busy_poll.h>
> @@ -2859,6 +2860,20 @@ int sock_get_timestampns(struct sock *sk, struct timespec __user *userstamp)
> }
> EXPORT_SYMBOL(sock_get_timestampns);
>
> +void sk_state_store(struct sock *sk, int newstate)
> +{
> + if (sk->sk_protocol == IPPROTO_TCP)
> + trace_tcp_set_state(sk, sk->sk_state, newstate);
I think this is going in the wrong way. When Dave said to not define a
sock generic function in tcp code on v3, you moved it all from tcp.h
to sock.h. But now sock.h gets to deal with more tcp code, which also
isn't nice.
Instead, if you move it back to tcp.h but rename the function to
tcp_state_store (instead of the original sk_state_store), it won't be
a generic sock code and will fit nicely into tcp.h.
You may then even keep sk_state_store() as it is now, and just define
tcp_state_store():
tcp_state_store()
{
trace_tcp_...();
sk_state_store();
}
Making it very clear that this code is only to be used by tcp stack.
> + __sk_state_store(sk, newstate);
> +}
> +
> +void sk_set_state(struct sock *sk, int state)
Same about this one.
Dave?
> +{
> + if (sk->sk_protocol == IPPROTO_TCP)
> + trace_tcp_set_state(sk, sk->sk_state, state);
> + sk->sk_state = state;
> +}
> +
> void sock_enable_timestamp(struct sock *sk, int flag)
> {
> if (!sock_flag(sk, flag)) {
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 4ca46dc..41f9c87 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -783,7 +783,7 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
> if (newsk) {
> struct inet_connection_sock *newicsk = inet_csk(newsk);
>
> - newsk->sk_state = TCP_SYN_RECV;
> + sk_set_state(newsk, TCP_SYN_RECV);
> newicsk->icsk_bind_hash = NULL;
>
> inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
> @@ -888,7 +888,8 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
> return 0;
> }
>
> - sk->sk_state = TCP_CLOSE;
> + sk_set_state(sk, TCP_CLOSE);
> +
> return err;
> }
> EXPORT_SYMBOL_GPL(inet_csk_listen_start);
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index f6f5810..5973693 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -544,7 +544,7 @@ bool inet_ehash_nolisten(struct sock *sk, struct sock *osk)
> sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
> } else {
> percpu_counter_inc(sk->sk_prot->orphan_count);
> - sk->sk_state = TCP_CLOSE;
> + sk_set_state(sk, TCP_CLOSE);
> sock_set_flag(sk, SOCK_DEAD);
> inet_csk_destroy_sock(sk);
> }
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 1803116..ac98dc6 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2065,7 +2065,7 @@ void tcp_set_state(struct sock *sk, int state)
> /* Change state AFTER socket is unhashed to avoid closed
> * socket sitting in hash tables.
> */
> - sk_state_store(sk, state);
> + __sk_state_store(sk, state);
>
> #ifdef STATE_TRACE
> SOCK_DEBUG(sk, "TCP sk=%p, State %s -> %s\n", sk, statename[oldstate], statename[state]);
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 8/8] net: tipc: remove unused hardirq.h
From: Yang Shi @ 2017-12-07 20:02 UTC (permalink / raw)
To: Jon Maloy, linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
linux-crypto@vger.kernel.org, netdev@vger.kernel.org, Ying Xue,
David S. Miller
In-Reply-To: <AM4PR07MB17147A3C4885EE59CFA58BDA9A330@AM4PR07MB1714.eurprd07.prod.outlook.com>
On 12/7/17 11:20 AM, Jon Maloy wrote:
>
>
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>> owner@vger.kernel.org] On Behalf Of Yang Shi
>> Sent: Thursday, December 07, 2017 14:16
>> To: linux-kernel@vger.kernel.org
>> Cc: linux-mm@kvack.org; linux-fsdevel@vger.kernel.org; linux-
>> crypto@vger.kernel.org; netdev@vger.kernel.org; Jon Maloy
>> <jon.maloy@ericsson.com>; Ying Xue <ying.xue@windriver.com>; David S.
>> Miller <davem@davemloft.net>
>> Subject: Re: [PATCH 8/8] net: tipc: remove unused hardirq.h
>>
>> Hi folks,
>>
>> Any comment on this one?
>
> If it compiles it is ok with me. Don't know why it was put there in the first place.
Yes, it does compile.
Yang
>
> ///jon
>
>>
>> Thanks,
>> Yang
>>
>>
>> On 11/17/17 3:02 PM, Yang Shi wrote:
>>> Preempt counter APIs have been split out, currently, hardirq.h just
>>> includes irq_enter/exit APIs which are not used by TIPC at all.
>>>
>>> So, remove the unused hardirq.h.
>>>
>>> Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
>>> Cc: Jon Maloy <jon.maloy@ericsson.com>
>>> Cc: Ying Xue <ying.xue@windriver.com>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>> ---
>>> net/tipc/core.h | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/net/tipc/core.h b/net/tipc/core.h index 5cc5398..099e072
>>> 100644
>>> --- a/net/tipc/core.h
>>> +++ b/net/tipc/core.h
>>> @@ -49,7 +49,6 @@
>>> #include <linux/uaccess.h>
>>> #include <linux/interrupt.h>
>>> #include <linux/atomic.h>
>>> -#include <asm/hardirq.h>
>>> #include <linux/netdevice.h>
>>> #include <linux/in.h>
>>> #include <linux/list.h>
>>>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net-next 0/6] cxgb4: collect hardware logs via ethtool
From: David Miller @ 2017-12-07 19:57 UTC (permalink / raw)
To: rahul.lakkireddy; +Cc: netdev, ganeshgr, nirranjan, indranil
In-Reply-To: <20171207.145606.1899010205120159321.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Thu, 07 Dec 2017 14:56:06 -0500 (EST)
> From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> Date: Thu, 7 Dec 2017 14:33:36 +0530
>
>> Collect more hardware logs via ethtool --get-dump facility.
>>
>> Patch 1 collects on-chip memory layout information.
>>
>> Patch 2 collects on-chip MC memory dumps.
>>
>> Patch 3 collects HMA memory dump.
>>
>> Patch 4 evaluates and skips TX and RX payload regions in memory dumps.
>>
>> Patch 5 collects egress and ingress SGE queue contexts.
>>
>> Patch 6 collects PCIe configuration logs
>
> Series applied.
Actually, reverted, please fix this new build warning:
drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c: In function ‘cudbg_fill_meminfo’:
drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c:261:33: warning: ‘size’ may be used uninitialized in this function [-Wmaybe-uninitialized]
md->limit = md->base + (size << 2) - 1;
~~~~~~^~~~~
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: correct error handling for chan create
From: Ivan Khoronzhuk @ 2017-12-07 19:56 UTC (permalink / raw)
To: David Miller; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171206.163814.2298864765495270615.davem@davemloft.net>
On Wed, Dec 06, 2017 at 04:38:14PM -0500, David Miller wrote:
> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Date: Wed, 6 Dec 2017 16:54:09 +0200
>
> > @@ -3065,10 +3065,16 @@ static int cpsw_probe(struct platform_device *pdev)
> > }
> >
> > cpsw->txv[0].ch = cpdma_chan_create(cpsw->dma, 0, cpsw_tx_handler, 0);
> > + if (WARN_ON(IS_ERR(cpsw->txv[0].ch))) {
> > + dev_err(priv->dev, "error initializing tx dma channel\n");
> > + ret = PTR_ERR(cpsw->txv[0].ch);
> > + goto clean_dma_ret;
> > + }
> > +
>
> You're already emitting a proper dev_err() message, therefore WARN_ON()
> is a duplicate notification to the logs and therefore not appropriate.
Yes, will remove unneeded WARNs
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net-next 0/6] cxgb4: collect hardware logs via ethtool
From: David Miller @ 2017-12-07 19:56 UTC (permalink / raw)
To: rahul.lakkireddy; +Cc: netdev, ganeshgr, nirranjan, indranil
In-Reply-To: <cover.1512636113.git.rahul.lakkireddy@chelsio.com>
From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Date: Thu, 7 Dec 2017 14:33:36 +0530
> Collect more hardware logs via ethtool --get-dump facility.
>
> Patch 1 collects on-chip memory layout information.
>
> Patch 2 collects on-chip MC memory dumps.
>
> Patch 3 collects HMA memory dump.
>
> Patch 4 evaluates and skips TX and RX payload regions in memory dumps.
>
> Patch 5 collects egress and ingress SGE queue contexts.
>
> Patch 6 collects PCIe configuration logs
Series applied.
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: mvpp2: only free the TSO header buffers when it was allocated
From: David Miller @ 2017-12-07 19:53 UTC (permalink / raw)
To: antoine.tenart
Cc: gregory.clement, thomas.petazzoni, miquel.raynal, nadavh, mw,
stefanc, ymarkman, netdev, linux-kernel
In-Reply-To: <20171207084903.27144-2-antoine.tenart@free-electrons.com>
From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Thu, 7 Dec 2017 09:48:58 +0100
> This patch adds a check to only free the TSO header buffer when its
> allocation previously succeeded.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
No, please keep this as a failure to bring up.
Even if you emit a log message, it is completely unintuitive to
have netdev features change on the user just because of a memory
allocation failure.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: rate is not changed - correct case
From: David Miller @ 2017-12-07 19:50 UTC (permalink / raw)
To: ivan.khoronzhuk; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171207194855.GA3022@khorivan>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Thu, 7 Dec 2017 21:48:56 +0200
> On Wed, Dec 06, 2017 at 04:35:45PM -0500, David Miller wrote:
>> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> Date: Wed, 6 Dec 2017 16:41:18 +0200
>>
>> > If rate is the same as set it's correct case.
>> >
>> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> > ---
>> > Based on net-next/master
>> >
>> > drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
>> > index e4d6edf..dbe9167 100644
>> > --- a/drivers/net/ethernet/ti/davinci_cpdma.c
>> > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
>> > @@ -841,7 +841,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
>> > return -EINVAL;
>> >
>> > if (ch->rate == rate)
>> > - return rate;
>> > + return 0;
>>
>> Looking at the one and only caller of this function, cpsw_ndo_set_tx_maxrate, it
>> makes sure this can never, ever, happen.
> In current circumstances yes, it will never happen.
> But I caught it while adding related code and better return 0 if upper caller
> doesn't have such check. Suppose that cpdma module is responsible for itself
> and if it's critical I can send this patch along with whole related series.
You have to decide one way or the other, who is responsible.
I think checking higher up is better because it's cheaper at that point to
look at the per-netdev queue rate setting before moving down deeper into the
driver specific data-structures.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: rate is not changed - correct case
From: Ivan Khoronzhuk @ 2017-12-07 19:48 UTC (permalink / raw)
To: David Miller; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171206.163545.906490902377440615.davem@davemloft.net>
On Wed, Dec 06, 2017 at 04:35:45PM -0500, David Miller wrote:
> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Date: Wed, 6 Dec 2017 16:41:18 +0200
>
> > If rate is the same as set it's correct case.
> >
> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> > ---
> > Based on net-next/master
> >
> > drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> > index e4d6edf..dbe9167 100644
> > --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> > @@ -841,7 +841,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
> > return -EINVAL;
> >
> > if (ch->rate == rate)
> > - return rate;
> > + return 0;
>
> Looking at the one and only caller of this function, cpsw_ndo_set_tx_maxrate, it
> makes sure this can never, ever, happen.
In current circumstances yes, it will never happen.
But I caught it while adding related code and better return 0 if upper caller
doesn't have such check. Suppose that cpdma module is responsible for itself
and if it's critical I can send this patch along with whole related series.
>
> So I would instead remove this check completely since it can never trigger.
--
Regards,
Ivan Khoronzhuk
^ 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