* [PATCH net-next 0/3] tcp: better TCP_SKB_CB layout
From: Eric Dumazet @ 2014-09-22 23:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Yuchung Cheng, Neal Cardwell, Eric Dumazet
TCP had the assumption that IPCB and IP6CB are first members of skb->cb[]
This is fine, except that IPCB/IP6CB are used for a very short time
in input path.
What really matters for TCP stack is to get skb->next,
TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq in the same cache line.
skb that are immediately consumed do not care because whole skb->cb[] is
hot in cpu cache, while skb that sit in wocket write queue or receive queues
do not need TCP_SKB_CB(skb)->header at all.
This patch set implements the prereq for IPv4, IPv6, and TCP to make this
possible. This makes TCP more efficient.
Eric Dumazet (3):
ipv4: rename ip_options_echo to __ip_options_echo()
ipv6: add a struct inet6_skb_parm param to ipv6_opt_accepted()
tcp: better TCP_SKB_CB layout to reduce cache line misses
include/net/ip.h | 15 ++++++++++++---
include/net/ipv6.h | 3 ++-
include/net/tcp.h | 12 ++++++------
net/dccp/ipv6.c | 2 +-
net/ipv4/ip_options.c | 6 ++----
net/ipv4/ip_output.c | 8 +++++---
net/ipv4/tcp_ipv4.c | 17 +++++++++++++----
net/ipv6/af_inet6.c | 4 ++--
net/ipv6/syncookies.c | 2 +-
net/ipv6/tcp_ipv6.c | 12 ++++++++++--
10 files changed, 54 insertions(+), 27 deletions(-)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply
* [PATCH net-next 1/3] ipv4: rename ip_options_echo to __ip_options_echo()
From: Eric Dumazet @ 2014-09-22 23:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Yuchung Cheng, Neal Cardwell, Eric Dumazet
In-Reply-To: <1411429844-11099-1-git-send-email-edumazet@google.com>
ip_options_echo() assumes struct ip_options is provided in &IPCB(skb)->opt
Lets break this assumption, but provide a helper to not change all call points.
ip_send_unicast_reply() gets a new struct ip_options pointer.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip.h | 15 ++++++++++++---
net/ipv4/ip_options.c | 6 ++----
net/ipv4/ip_output.c | 8 +++++---
net/ipv4/tcp_ipv4.c | 10 ++++++----
4 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 14bfc8e1bcf9..c90e3357d589 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -180,8 +180,10 @@ static inline __u8 ip_reply_arg_flowi_flags(const struct ip_reply_arg *arg)
return (arg->flags & IP_REPLY_ARG_NOSRCCHECK) ? FLOWI_FLAG_ANYSRC : 0;
}
-void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
- __be32 saddr, const struct ip_reply_arg *arg,
+void ip_send_unicast_reply(struct net *net, struct sk_buff *skb,
+ const struct ip_options *sopt,
+ __be32 daddr, __be32 saddr,
+ const struct ip_reply_arg *arg,
unsigned int len);
#define IP_INC_STATS(net, field) SNMP_INC_STATS64((net)->mib.ip_statistics, field)
@@ -511,7 +513,14 @@ int ip_forward(struct sk_buff *skb);
void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
__be32 daddr, struct rtable *rt, int is_frag);
-int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb);
+
+int __ip_options_echo(struct ip_options *dopt, struct sk_buff *skb,
+ const struct ip_options *sopt);
+static inline int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb)
+{
+ return __ip_options_echo(dopt, skb, &IPCB(skb)->opt);
+}
+
void ip_options_fragment(struct sk_buff *skb);
int ip_options_compile(struct net *net, struct ip_options *opt,
struct sk_buff *skb);
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index ad382499bace..5b3d91be2db0 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -87,17 +87,15 @@ void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
* NOTE: dopt cannot point to skb.
*/
-int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb)
+int __ip_options_echo(struct ip_options *dopt, struct sk_buff *skb,
+ const struct ip_options *sopt)
{
- const struct ip_options *sopt;
unsigned char *sptr, *dptr;
int soffset, doffset;
int optlen;
memset(dopt, 0, sizeof(struct ip_options));
- sopt = &(IPCB(skb)->opt);
-
if (sopt->optlen == 0)
return 0;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 215af2b155cb..c8fa62476461 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1522,8 +1522,10 @@ static DEFINE_PER_CPU(struct inet_sock, unicast_sock) = {
.uc_ttl = -1,
};
-void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
- __be32 saddr, const struct ip_reply_arg *arg,
+void ip_send_unicast_reply(struct net *net, struct sk_buff *skb,
+ const struct ip_options *sopt,
+ __be32 daddr, __be32 saddr,
+ const struct ip_reply_arg *arg,
unsigned int len)
{
struct ip_options_data replyopts;
@@ -1534,7 +1536,7 @@ void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
struct sock *sk;
struct inet_sock *inet;
- if (ip_options_echo(&replyopts.opt.opt, skb))
+ if (__ip_options_echo(&replyopts.opt.opt, skb, sopt))
return;
ipc.addr = daddr;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3b2e49cb2b61..28ab90382c01 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -681,8 +681,9 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
net = dev_net(skb_dst(skb)->dev);
arg.tos = ip_hdr(skb)->tos;
- ip_send_unicast_reply(net, skb, ip_hdr(skb)->saddr,
- ip_hdr(skb)->daddr, &arg, arg.iov[0].iov_len);
+ ip_send_unicast_reply(net, skb, &TCP_SKB_CB(skb)->header.h4.opt,
+ ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
+ &arg, arg.iov[0].iov_len);
TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
@@ -764,8 +765,9 @@ static void tcp_v4_send_ack(struct sk_buff *skb, u32 seq, u32 ack,
if (oif)
arg.bound_dev_if = oif;
arg.tos = tos;
- ip_send_unicast_reply(net, skb, ip_hdr(skb)->saddr,
- ip_hdr(skb)->daddr, &arg, arg.iov[0].iov_len);
+ ip_send_unicast_reply(net, skb, &TCP_SKB_CB(skb)->header.h4.opt,
+ ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
+ &arg, arg.iov[0].iov_len);
TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH net-next 2/3] ipv6: add a struct inet6_skb_parm param to ipv6_opt_accepted()
From: Eric Dumazet @ 2014-09-22 23:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Yuchung Cheng, Neal Cardwell, Eric Dumazet
In-Reply-To: <1411429844-11099-1-git-send-email-edumazet@google.com>
ipv6_opt_accepted() assumes IP6CB(skb) holds the struct inet6_skb_parm
that it needs. Lets not assume this, as TCP stack might use a different
place.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ipv6.h | 3 ++-
net/dccp/ipv6.c | 2 +-
net/ipv6/af_inet6.c | 4 ++--
net/ipv6/syncookies.c | 2 +-
net/ipv6/tcp_ipv6.c | 5 +++--
5 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 7e247e9b8765..97f472012438 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -288,7 +288,8 @@ struct ipv6_txoptions *ipv6_renew_options(struct sock *sk,
struct ipv6_txoptions *ipv6_fixup_options(struct ipv6_txoptions *opt_space,
struct ipv6_txoptions *opt);
-bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb);
+bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb,
+ const struct inet6_skb_parm *opt);
static inline bool ipv6_accept_ra(struct inet6_dev *idev)
{
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 04cb17d4b0ce..ad2acfe1ca61 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -404,7 +404,7 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
- if (ipv6_opt_accepted(sk, skb) ||
+ if (ipv6_opt_accepted(sk, skb, IP6CB(skb)) ||
np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim) {
atomic_inc(&skb->users);
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index e4865a3ebe1d..34f726f59814 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -672,10 +672,10 @@ int inet6_sk_rebuild_header(struct sock *sk)
}
EXPORT_SYMBOL_GPL(inet6_sk_rebuild_header);
-bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb)
+bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb,
+ const struct inet6_skb_parm *opt)
{
const struct ipv6_pinfo *np = inet6_sk(sk);
- const struct inet6_skb_parm *opt = IP6CB(skb);
if (np->rxopt.all) {
if ((opt->hop && (np->rxopt.bits.hopopts ||
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index c643dc907ce7..9a2838e93cc5 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -203,7 +203,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
ireq->ir_num = ntohs(th->dest);
ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
- if (ipv6_opt_accepted(sk, skb) ||
+ if (ipv6_opt_accepted(sk, skb, &TCP_SKB_CB(skb)->header.h6) ||
np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim) {
atomic_inc(&skb->users);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index de51a88bec6f..9400b4326f22 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -742,7 +742,8 @@ static void tcp_v6_init_req(struct request_sock *req, struct sock *sk,
ireq->ir_iif = inet6_iif(skb);
if (!TCP_SKB_CB(skb)->tcp_tw_isn &&
- (ipv6_opt_accepted(sk, skb) || np->rxopt.bits.rxinfo ||
+ (ipv6_opt_accepted(sk, skb, &TCP_SKB_CB(skb)->header.h6) ||
+ np->rxopt.bits.rxinfo ||
np->rxopt.bits.rxoinfo || np->rxopt.bits.rxhlim ||
np->rxopt.bits.rxohlim || np->repflow)) {
atomic_inc(&skb->users);
@@ -1367,7 +1368,7 @@ ipv6_pktoptions:
np->rcv_flowinfo = ip6_flowinfo(ipv6_hdr(opt_skb));
if (np->repflow)
np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
- if (ipv6_opt_accepted(sk, opt_skb)) {
+ if (ipv6_opt_accepted(sk, opt_skb, &TCP_SKB_CB(opt_skb)->header.h6)) {
skb_set_owner_r(opt_skb, sk);
opt_skb = xchg(&np->pktoptions, opt_skb);
} else {
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH net-next 3/3] tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Eric Dumazet @ 2014-09-22 23:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Yuchung Cheng, Neal Cardwell, Eric Dumazet
In-Reply-To: <1411429844-11099-1-git-send-email-edumazet@google.com>
TCP maintains lists of skb in write queue, and in receive queues
(in order and out of order queues)
Scanning these lists both in input and output path usually requires
access to skb->next, TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq
These fields are currently in two different cache lines, meaning we
waste lot of memory bandwidth when these queues are big and flows
have either packet drops or packet reorders.
We can move TCP_SKB_CB(skb)->header at the end of TCP_SKB_CB, because
this header is not used in fast path. This allows TCP to search much faster
in the skb lists.
Even with regular flows, we save one cache line miss in fast path.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/tcp.h | 12 ++++++------
net/ipv4/tcp_ipv4.c | 7 +++++++
net/ipv6/tcp_ipv6.c | 7 +++++++
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index a4201ef216e8..4dc6641ee990 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -696,12 +696,6 @@ static inline u32 tcp_skb_timestamp(const struct sk_buff *skb)
* If this grows please adjust skbuff.h:skbuff->cb[xxx] size appropriately.
*/
struct tcp_skb_cb {
- union {
- struct inet_skb_parm h4;
-#if IS_ENABLED(CONFIG_IPV6)
- struct inet6_skb_parm h6;
-#endif
- } header; /* For incoming frames */
__u32 seq; /* Starting sequence number */
__u32 end_seq; /* SEQ + FIN + SYN + datalen */
__u32 tcp_tw_isn; /* isn chosen by tcp_timewait_state_process() */
@@ -720,6 +714,12 @@ struct tcp_skb_cb {
__u8 ip_dsfield; /* IPv4 tos or IPv6 dsfield */
/* 1 byte hole */
__u32 ack_seq; /* Sequence number ACK'd */
+ union {
+ struct inet_skb_parm h4;
+#if IS_ENABLED(CONFIG_IPV6)
+ struct inet6_skb_parm h6;
+#endif
+ } header; /* For incoming frames */
};
#define TCP_SKB_CB(__skb) ((struct tcp_skb_cb *)&((__skb)->cb[0]))
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 28ab90382c01..70c4a21f6f45 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1636,6 +1636,13 @@ int tcp_v4_rcv(struct sk_buff *skb)
th = tcp_hdr(skb);
iph = ip_hdr(skb);
+ /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+ * barrier() makes sure compiler wont play fool^Waliasing games.
+ */
+ memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
+ sizeof(struct inet_skb_parm));
+ barrier();
+
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
skb->len - th->doff * 4);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9400b4326f22..132bac137aed 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1412,6 +1412,13 @@ static int tcp_v6_rcv(struct sk_buff *skb)
th = tcp_hdr(skb);
hdr = ipv6_hdr(skb);
+ /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+ * barrier() makes sure compiler wont play fool^Waliasing games.
+ */
+ memmove(&TCP_SKB_CB(skb)->header.h6, IP6CB(skb),
+ sizeof(struct inet6_skb_parm));
+ barrier();
+
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
skb->len - th->doff*4);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Angus Gibson @ 2014-09-22 23:56 UTC (permalink / raw)
To: mark.einon; +Cc: linux-kernel, netdev, devel
In-Reply-To: <1411421283-25073-1-git-send-email-mark.einon@gmail.com>
> +config NET_VENDOR_AGERE
> + bool "Agere devices"
> + default y
> + depends on PCI
> + ---help---
> + If you have a network (Ethernet) card belonging to this class, say =
> Y
> + and read the Ethernet-HOWTO, available from
> + <http://www.tldp.org/docs.html#howto>.
> +
> + Note that the answer to this question doesn't directly affect the
> + kernel: saying N will just cause the configurator to skip all
> + the questions about Atheros devices. If you say Y, you will be aske=
should this be "Agere devices" instead?
> d
> + for your specific card in the following questions.
^ permalink raw reply
* RE: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected
From: Kweh, Hock Leong @ 2014-09-23 1:16 UTC (permalink / raw)
To: David Miller
Cc: peppe.cavallaro@st.com, rayagond@vayavyalabs.com,
vbridgers2013@gmail.com, wens@csie.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Ong, Boon Leong,
tobias.johannes.klausmann@mni.thm.de
In-Reply-To: <20140922.141921.577008114679288180.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, September 23, 2014 2:19 AM
> From: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Date: Thu, 18 Sep 2014 20:34:10 +0800
>
> Giuseppe, Kweh, where are we with this patch?
We are discussing whether this is the correct fix to the issue. Below is the discussion log:
> > Hmm I am not sure this is the right fix. The driver has to fail if the
> > main clock is not found. Indeed dev_warn has to be changed in dev_err.
> >
> > Take a look at Documentation/networking/stmmac.txt but I will post
> > some patch to improve the documentation adding further detail for clocks too.
> >
> > The the logic behind the code is that the CSR clock will be set at
> > runtime if in case of priv->plat->clk_csr ==0 or it will be forced to
> > a fixed value if passed from the platform instead of.
> > IIRC This was required on some platforms time ago.
> > For sure the driver is designed to fail in case of no main clock is found.
> >
> > Peppe
>
> Hi Peppe,
>
> I understand your point from the code below (at file stmmac_main.c line 2784):
>
> /* If a specific clk_csr value is passed from the platform
> * this means that the CSR Clock Range selection cannot be
> * changed at run-time and it is fixed. Viceversa the driver'll try to
> * set the MDC clock dynamically according to the csr actual
> * clock input.
> */
> if (!priv->plat->clk_csr)
> stmmac_clk_csr_set(priv);
> else
> priv->clk_csr = priv->plat->clk_csr;
>
>
> I did search through the whole stmmac_main.c file and found that only stmmac_clk_csr_set()
> function is leveraging the priv->stmmac_clk params for it calculation. By the logic point of view,
> I do not need priv->stmmac_clk when I got priv->plat->clk_csr. With this thinking, I propose this
> fix as when the probe get priv->plat->clk_csr, it shouldn't fail if priv->stmmac_clk has the error value.
Hi Peppe,
Are you trying to tell that if there is a fix CSR clock value, but driver cannot obtain the clk from devm_clk_get()
(even it is not in use), the driver should fail the probe?
You have a case with this condition:
HW allow SW to discover the STMMAC controller but HW/SW configures not to supply the CLOCK to STMMAC controller?
My understanding to these priv->plat->clk_csr and priv->stmmac_clk is that 1st one is fixed and 2nd one is dynamic.
When fixed value occurs, dynamic one would not be use. Do I understand this correctly?
Thanks.
Regards,
Wilson
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: John Fastabend @ 2014-09-23 1:36 UTC (permalink / raw)
To: Tom Herbert
Cc: Thomas Graf, Alexei Starovoitov, Jiri Pirko, John Fastabend,
Jamal Hadi Salim, netdev@vger.kernel.org, David S. Miller,
Neil Horman, Andy Gospodarek, Daniel Borkmann, Or Gerlitz,
Jesse Gross, Pravin Shelar, Andy Zhou, Ben Hutchings,
Stephen Hemminger, Jeff Kirsher, Vladislav Yasevich, Cong Wang,
Eric Dumazet, Scott Feldman, Florian Fainelli, Roopa
In-Reply-To: <CA+mtBx94TrLzQrZDo58d9ktma3ZioOpO=2FAODMRoiyiuYkQ9Q@mail.gmail.com>
On 09/22/2014 04:07 PM, Tom Herbert wrote:
> On Mon, Sep 22, 2014 at 3:53 PM, Thomas Graf <tgraf@suug.ch> wrote:
>> On 09/22/14 at 03:40pm, Tom Herbert wrote:
>>> On Mon, Sep 22, 2014 at 3:17 PM, Thomas Graf <tgraf@suug.ch> wrote:
>>>> What makes stateful offload interesting to me is that the final
>>>> desintation of a packet is known at RX and can be redirected to a
>>>> queue or VF. This allows to build packet batches on shared pages
>>>> while preserving the securiy model.
>>>>
>>> How is this different from what rx-filtering already does?
>>
>> Without stateful offload I can't know where the packet is destined
>> to until after I've allocated an skb and parsed the packet in
>> software. I might be missing what you refer to here specifically.
>
> n-tuple filtering in as exposed by ethtool.
n-tuple has some deficiencies,
- its not possible to get the capabilities to learn what
fields are supported by the device, what actions, etc.
- its ioctl based so we have to poll the device
- only supports a single table, where we have devices with
multiple tables
- sort of the same as above but it doesn't allow creating new
tables or destroying old tables.
I probably missed a few others but those are the main ones that I
would like to address. Granted other than the ioctl line the rest could
be solved by extending the existing interface. However I would just
assume port it to ndo_ops and netlink then extend the existing ioctl
seeing it needs a reasonable overall to support the above anyways.
We could port the ethtool ops over to the new interface to
simplify drivers.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Alexei Starovoitov @ 2014-09-23 1:54 UTC (permalink / raw)
To: Tom Herbert
Cc: Thomas Graf, Jiri Pirko, John Fastabend, Jamal Hadi Salim,
netdev@vger.kernel.org, David S. Miller, Neil Horman,
Andy Gospodarek, Daniel Borkmann, Or Gerlitz, Jesse Gross,
Pravin Shelar, Andy Zhou, Ben Hutchings, Stephen Hemminger,
Jeff Kirsher, Vladislav Yasevich, Cong Wang, Eric Dumazet,
Scott Feldman, Florian Fainelli, Roopa Prabhu,
John Linville <linvi
In-Reply-To: <CA+mtBx-ZvkZ2ALGLsLEN7Cgn9gN_rY36ZhuqYC3mZ9WVxjFDaQ@mail.gmail.com>
On Mon, Sep 22, 2014 at 8:10 AM, Tom Herbert <therbert@google.com> wrote:
> On Mon, Sep 22, 2014 at 1:13 AM, Thomas Graf <tgraf@suug.ch> wrote:
>> On 09/20/14 at 03:50pm, Alexei Starovoitov wrote:
>>> I think HW should not be limited by SW abstractions whether
>>> these abstractions are called flows, n-tuples, bridge or else.
>>> Really looking forward to see "device reporting the headers as
>>> header fields (len, offset) and the associated parse graph"
>>> as the first step.
>>>
>>> Another topic that this discussion didn't cover yet is how this
>>> all connects to tunnels and what is 'tunnel offloading'.
> encapsulation (stuffing a few bytes of header into a packet) is in
> itself not nearly an expensive enough operation to warrant offloading
> to the NIC. Personally, I wish if NIC vendors are going to focus on
On contrary, generic tunneling is most important one to get right
when we're talking offloads.
Adding encap header is easy to do in hw, but it breaks all other
offloads if hw is not generic. Consider gso packet coming from vm.
Generic tunnel allows sw to add inner headers, outer headers and
setup offload offsets, so that HW does segmentation, checksuming
of inner packet, adjusts inner headers and adds final outer encap.
And this is just tx offload. On rx smart tunnel offload in HW parses
encap and goes all the way to inner headers to verify checksums,
it also steers based on inner headers.
Try mellanox nics with and without vxlan offload to see
the difference.
It looks like fm10k will be just as good, but existing encaps are
not going to last forever, so RX should be improved they way John
is saying. There gotta to be a 'parse graph' for HW to see past
variable length encap and into inner headers.
checksum_complete style of offloading checksum verification
is not efficient. The cost of adjusting it over and over while
parsing encaps is too high. Plus cpu steering based on outer
headers is just too slow when speeds are in 40G range.
> stateful offload I rather see it be for encryption which I believe
> currently does warrant offload at 40G and higher speeds.
encryption offload is badly needed as well. Unfortunately it's
not seen as nic feature yet.
^ permalink raw reply
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Joe Perches @ 2014-09-23 1:57 UTC (permalink / raw)
To: Mark Einon; +Cc: gregkh, davem, linux-kernel, netdev, devel
In-Reply-To: <1411421283-25073-1-git-send-email-mark.einon@gmail.com>
On Mon, 2014-09-22 at 22:28 +0100, Mark Einon wrote:
> This patch moves the et131x gigabit ethernet driver from drivers/staging
> to drivers/net/ethernet/agere.
>
> There are no known issues at this time.
This doesn't apply to -next or -linux
[]
> This patch will only apply once the last few pending changes
> make their way from staging-next to linux-next, but posting
> now in the hope that feedback can be given and this change can
> make it in before the next merge window.n yo
When you do post the actual patch, can you please use
"git format-patch -M" so that the patch is significantly
smaller and the renames are obvious?
> drivers/net/ethernet/Kconfig | 1 +
> drivers/net/ethernet/Makefile | 1 +
> drivers/net/ethernet/agere/Kconfig | 31 +
> drivers/net/ethernet/agere/Makefile | 5 +
> drivers/net/ethernet/agere/et131x.c | 4412 +++++++++++++++++++++++++++++++++++
> drivers/net/ethernet/agere/et131x.h | 1533 ++++++++++++
> drivers/staging/Kconfig | 2 -
> drivers/staging/Makefile | 1 -
> drivers/staging/et131x/Kconfig | 10 -
> drivers/staging/et131x/Makefile | 5 -
> drivers/staging/et131x/README | 15 -
> drivers/staging/et131x/et131x.c | 4412 -----------------------------------
> drivers/staging/et131x/et131x.h | 1533 ------------
> 13 files changed, 5983 insertions(+), 5978 deletions(-)
> create mode 100644 drivers/net/ethernet/agere/Kconfig
> create mode 100644 drivers/net/ethernet/agere/Makefile
> create mode 100644 drivers/net/ethernet/agere/et131x.c
> create mode 100644 drivers/net/ethernet/agere/et131x.h
> delete mode 100644 drivers/staging/et131x/Kconfig
> delete mode 100644 drivers/staging/et131x/Makefile
> delete mode 100644 drivers/staging/et131x/README
> delete mode 100644 drivers/staging/et131x/et131x.c
> delete mode 100644 drivers/staging/et131x/et131x.h
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Tom Herbert @ 2014-09-23 2:16 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Thomas Graf, Jiri Pirko, John Fastabend, Jamal Hadi Salim,
netdev@vger.kernel.org, David S. Miller, Neil Horman,
Andy Gospodarek, Daniel Borkmann, Or Gerlitz, Jesse Gross,
Pravin Shelar, Andy Zhou, Ben Hutchings, Stephen Hemminger,
Jeff Kirsher, Vladislav Yasevich, Cong Wang, Eric Dumazet,
Scott Feldman, Florian Fainelli, Roopa Prabhu,
John Linville <linvi
In-Reply-To: <CAADnVQKhUAR=qVg9q82ngYMzQnoDVbq+wupzdMrH8WiCeMiU4A@mail.gmail.com>
On Mon, Sep 22, 2014 at 6:54 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Mon, Sep 22, 2014 at 8:10 AM, Tom Herbert <therbert@google.com> wrote:
>> On Mon, Sep 22, 2014 at 1:13 AM, Thomas Graf <tgraf@suug.ch> wrote:
>>> On 09/20/14 at 03:50pm, Alexei Starovoitov wrote:
>>>> I think HW should not be limited by SW abstractions whether
>>>> these abstractions are called flows, n-tuples, bridge or else.
>>>> Really looking forward to see "device reporting the headers as
>>>> header fields (len, offset) and the associated parse graph"
>>>> as the first step.
>>>>
>>>> Another topic that this discussion didn't cover yet is how this
>>>> all connects to tunnels and what is 'tunnel offloading'.
>
>> encapsulation (stuffing a few bytes of header into a packet) is in
>> itself not nearly an expensive enough operation to warrant offloading
>> to the NIC. Personally, I wish if NIC vendors are going to focus on
>
> On contrary, generic tunneling is most important one to get right
> when we're talking offloads.
> Adding encap header is easy to do in hw, but it breaks all other
> offloads if hw is not generic. Consider gso packet coming from vm.
> Generic tunnel allows sw to add inner headers, outer headers and
> setup offload offsets, so that HW does segmentation, checksuming
> of inner packet, adjusts inner headers and adds final outer encap.
As I pointed out on a previous thread, we already have a sufficiently
generic interface to allow HW to do encapsulated TSO
(SKB_GSO_UDP_TUNNEL and SKB_GSO_UDP_TUNNEL_CSUM with the inner
headers). If properly implemented, HW can implement a whole bunch of
UDP encap protocols without knowing how to parse them. I don't see how
a switch on the NIC helps this...
> And this is just tx offload. On rx smart tunnel offload in HW parses
> encap and goes all the way to inner headers to verify checksums,
> it also steers based on inner headers.
> Try mellanox nics with and without vxlan offload to see
> the difference.
Turn on UDP RSS on the device and I bet you'll see those differences
go away! Once we moved to UDP encapsulation, there's really little
value in looking at inner headers for RSS or ECMP, this should be
sufficient. Sure someone might want to parse the inner headers for
some sort of advanced RX steering, but again this implies rx-filtering
and not switch functionality.
Alexei, I believe you said previously said that SW should not dictate
HW models. I agree with this, but also believe the converse is true--
HW shouldn't dictate SW model. This is really why I'm raising the
question of what it means to integrate a switch into the host stack.
If this is something that doesn't require any model change to the
stack and is just a clever backend for rx-filters or tc, then I'm fine
with that!
Thanks,
Tom
> It looks like fm10k will be just as good, but existing encaps are
> not going to last forever, so RX should be improved they way John
> is saying. There gotta to be a 'parse graph' for HW to see past
> variable length encap and into inner headers.
> checksum_complete style of offloading checksum verification
> is not efficient. The cost of adjusting it over and over while
> parsing encaps is too high. Plus cpu steering based on outer
> headers is just too slow when speeds are in 40G range.
>
>> stateful offload I rather see it be for encryption which I believe
>> currently does warrant offload at 40G and higher speeds.
>
> encryption offload is badly needed as well. Unfortunately it's
> not seen as nic feature yet.
^ permalink raw reply
* Re: [PATCH] macvtap: Fix race between device delete and open.
From: Jason Wang @ 2014-09-23 2:56 UTC (permalink / raw)
To: Vladislav Yasevich, netdev; +Cc: Vladislav Yasevich, Michael S. Tsirkin
In-Reply-To: <1411418057-18937-1-git-send-email-vyasevic@redhat.com>
On 09/23/2014 04:34 AM, Vladislav Yasevich wrote:
> In macvtap device delete and open calls can race and
> this causes a list curruption of the vlan queue_list.
>
> The race intself is triggered by the idr accessors
> that located the vlan device. The device is stored
> into and removed from the idr under both an rtnl and
> a mutex. However, when attempting to locate the device
> in idr, only a mutex is taken. As a result, once cpu
> perfoming a delete may take an rtnl and wait for the mutex,
> while another cput doing an open() will take the idr
> mutex first to fetch the device pointer and later take
> an rtnl to add a queue for the device which may have
> just gotten deleted.
>
> With this patch, we now hold the rtnl for the duration
> of the macvtap_open() call thus making sure that
> open will not race with delete.
>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
> drivers/net/macvtap.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 3381c4f..0c6adaa 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -112,17 +112,15 @@ out:
> return err;
> }
>
> +/* Requires RTNL */
> static int macvtap_set_queue(struct net_device *dev, struct file *file,
> struct macvtap_queue *q)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> - int err = -EBUSY;
>
> - rtnl_lock();
> if (vlan->numqueues == MAX_MACVTAP_QUEUES)
> - goto out;
> + return -EBUSY;
>
> - err = 0;
> rcu_assign_pointer(q->vlan, vlan);
> rcu_assign_pointer(vlan->taps[vlan->numvtaps], q);
> sock_hold(&q->sk);
> @@ -136,9 +134,7 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
> vlan->numvtaps++;
> vlan->numqueues++;
>
> -out:
> - rtnl_unlock();
> - return err;
> + return 0;
> }
>
> static int macvtap_disable_queue(struct macvtap_queue *q)
> @@ -454,11 +450,12 @@ static void macvtap_sock_destruct(struct sock *sk)
> static int macvtap_open(struct inode *inode, struct file *file)
> {
> struct net *net = current->nsproxy->net_ns;
> - struct net_device *dev = dev_get_by_macvtap_minor(iminor(inode));
> + struct net_device *dev;
> struct macvtap_queue *q;
> - int err;
> + int err = -ENODEV;
>
> - err = -ENODEV;
> + rtnl_lock();
> + dev = dev_get_by_macvtap_minor(iminor(inode));
> if (!dev)
> goto out;
>
> @@ -498,6 +495,7 @@ out:
> if (dev)
> dev_put(dev);
>
> + rtnl_unlock();
> return err;
> }
>
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Alexei Starovoitov @ 2014-09-23 3:43 UTC (permalink / raw)
To: Tom Herbert
Cc: Thomas Graf, Jiri Pirko, John Fastabend, Jamal Hadi Salim,
netdev@vger.kernel.org, David S. Miller, Neil Horman,
Andy Gospodarek, Daniel Borkmann, Or Gerlitz, Jesse Gross,
Pravin Shelar, Andy Zhou, Ben Hutchings, Stephen Hemminger,
Jeff Kirsher, Vladislav Yasevich, Cong Wang, Eric Dumazet,
Scott Feldman, Florian Fainelli, Roopa Prabhu,
John Linville <linvi
On Mon, Sep 22, 2014 at 7:16 PM, Tom Herbert <therbert@google.com> wrote:
> On Mon, Sep 22, 2014 at 6:54 PM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Mon, Sep 22, 2014 at 8:10 AM, Tom Herbert <therbert@google.com> wrote:
>>> On Mon, Sep 22, 2014 at 1:13 AM, Thomas Graf <tgraf@suug.ch> wrote:
>>>> On 09/20/14 at 03:50pm, Alexei Starovoitov wrote:
>>>>> I think HW should not be limited by SW abstractions whether
>>>>> these abstractions are called flows, n-tuples, bridge or else.
>>>>> Really looking forward to see "device reporting the headers as
>>>>> header fields (len, offset) and the associated parse graph"
>>>>> as the first step.
>>>>>
>>>>> Another topic that this discussion didn't cover yet is how this
>>>>> all connects to tunnels and what is 'tunnel offloading'.
>>
>>> encapsulation (stuffing a few bytes of header into a packet) is in
>>> itself not nearly an expensive enough operation to warrant offloading
>>> to the NIC. Personally, I wish if NIC vendors are going to focus on
>>
>> On contrary, generic tunneling is most important one to get right
>> when we're talking offloads.
>> Adding encap header is easy to do in hw, but it breaks all other
>> offloads if hw is not generic. Consider gso packet coming from vm.
>> Generic tunnel allows sw to add inner headers, outer headers and
>> setup offload offsets, so that HW does segmentation, checksuming
>> of inner packet, adjusts inner headers and adds final outer encap.
>
> As I pointed out on a previous thread, we already have a sufficiently
> generic interface to allow HW to do encapsulated TSO
> (SKB_GSO_UDP_TUNNEL and SKB_GSO_UDP_TUNNEL_CSUM with the inner
> headers).
SKB_GSO_UDP_TUNNEL_CSUM was the right way
to start splitting overloaded and messy semantics of
UDP_TUNNEL. I'm still not sure whether you've intended
it for both rx and tx, since to support tunnel_csum on rx,
parsing of encap is needed, whereas tx is so much simpler.
Unless you're assuming checksum_complete model for rx...
> If properly implemented, HW can implement a whole bunch of
> UDP encap protocols without knowing how to parse them.
on a tx side... yes, but I cannot see how you can do rx
with inner csum verify without parsing encap.
What do you have in mind ?
> I don't see how
> a switch on the NIC helps this...
correct, just a switch on a nic isn't very useful.
If immediate consumer of the packet is a VM,
then doing switching in the nic after decap doesn't
add much speed, since bridge+router+nat+policy in sw
after decap and csum verify done by hw are fast enough.
But switching in HW becomes useful when VF
is a destination device, since it avoids hw->sw->hw
roundtrip as Thomas was saying.
Also there are x86 network gateways where tunneled
traffic from virtual network is terminated and sent
over internet or to other datacenter. Performance
demands are high, so if tunnel+switch+nat+policy
can be done in off-the-shelf HW it would be great.
>> And this is just tx offload. On rx smart tunnel offload in HW parses
>> encap and goes all the way to inner headers to verify checksums,
>> it also steers based on inner headers.
>> Try mellanox nics with and without vxlan offload to see
>> the difference.
>
> Turn on UDP RSS on the device and I bet you'll see those differences
> go away!
Logically it should, since all inner flows should get
hashed into different outer src_port, but somehow
that didn't work. Need to re-investigate with your
l4_hash stuff.
> Alexei, I believe you said previously said that SW should not dictate
> HW models. I agree with this, but also believe the converse is true--
> HW shouldn't dictate SW model.
completely agree!
> This is really why I'm raising the
> question of what it means to integrate a switch into the host stack.
> If this is something that doesn't require any model change to the
> stack and is just a clever backend for rx-filters or tc, then I'm fine
> with that!
agree as well. I'm not excited about switchdev
abstraction from this given patch, since it looks overly
simplified and not applicable to real silicon, but
discussion about exposing programmable
nics/switches to sw in a generic way is worth having :)
^ permalink raw reply
* [PATCH net-next] tcp: abort orphan sockets stalling on zero window probes
From: Yuchung Cheng @ 2014-09-23 3:52 UTC (permalink / raw)
To: davem; +Cc: edumazet, andrey.dmitrov, ncardwell, netdev, Yuchung Cheng
Currently we have two different policies for orphan sockets
that repeatedly stall on zero window ACKs. If a socket gets
a zero window ACK when it is transmitting data, the RTO is
used to probe the window. The socket is aborted after roughly
tcp_orphan_retries() retries (as in tcp_write_timeout()).
But if the socket was idle when it received the zero window ACK,
and later wants to send more data, we use the probe timer to
probe the window. If the receiver always returns zero window ACKs,
icsk_probes keeps getting reset in tcp_ack() and the orphan socket
can stall forever until the system reaches the orphan limit (as
commented in tcp_probe_timer()). This opens up a simple attack
to create lots of hanging orphan sockets to burn the memory
and the CPU, as demonstrated in the recent netdev post "TCP
connection will hang in FIN_WAIT1 after closing if zero window is
advertised." http://www.spinics.net/lists/netdev/msg296539.html
This patch follows the design in RTO-based probe: we abort an orphan
socket stalling on zero window when the probe timer reaches both
the maximum backoff and the maximum RTO. For example, an 100ms RTT
connection will timeout after roughly 153 seconds (0.3 + 0.6 +
.... + 76.8) if the receiver keeps the window shut.
In addition, we change TCP_USER_TIMEOUT to cover (life or dead)
sockets stalled on zero-window probes. This changes the semantics
of TCP_USER_TIMEOUT slightly because it previously only applies
when the socket has pending transmission.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru>
---
net/ipv4/tcp.c | 2 +-
net/ipv4/tcp_timer.c | 35 ++++++++++++++++++-----------------
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 070aeff..965a197 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2693,7 +2693,7 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
break;
#endif
case TCP_USER_TIMEOUT:
- /* Cap the max timeout in ms TCP will retry/retrans
+ /* Cap the max time in ms TCP will retry or probe the window
* before giving up and aborting (ETIMEDOUT) a connection.
*/
if (val < 0)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index b24360f..aa6773a 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -270,40 +270,41 @@ static void tcp_probe_timer(struct sock *sk)
struct inet_connection_sock *icsk = inet_csk(sk);
struct tcp_sock *tp = tcp_sk(sk);
int max_probes;
+ u32 start_ts;
if (tp->packets_out || !tcp_send_head(sk)) {
icsk->icsk_probes_out = 0;
return;
}
- /* *WARNING* RFC 1122 forbids this
- *
- * It doesn't AFAIK, because we kill the retransmit timer -AK
- *
- * FIXME: We ought not to do it, Solaris 2.5 actually has fixing
- * this behaviour in Solaris down as a bug fix. [AC]
- *
- * Let me to explain. icsk_probes_out is zeroed by incoming ACKs
- * even if they advertise zero window. Hence, connection is killed only
- * if we received no ACKs for normal connection timeout. It is not killed
- * only because window stays zero for some time, window may be zero
- * until armageddon and even later. We are in full accordance
- * with RFCs, only probe timer combines both retransmission timeout
- * and probe timeout in one bottle. --ANK
+ /* RFC 1122 4.2.2.17 requires the sender to stay open indefinitely as
+ * long as the receiver continues to respond probes. We support this by
+ * default and reset icsk_probes_out with incoming ACKs. But if the
+ * socket is orphaned or the user specifies TCP_USER_TIMEOUT, we
+ * kill the socket when the retry count and the time exceeds the
+ * corresponding system limit. We also implement similar policy when
+ * we use RTO to probe window in tcp_retransmit_timer().
*/
- max_probes = sysctl_tcp_retries2;
+ start_ts = tcp_skb_timestamp(tcp_send_head(sk));
+ if (!start_ts)
+ skb_mstamp_get(&tcp_send_head(sk)->skb_mstamp);
+ else if (icsk->icsk_user_timeout &&
+ (s32)(tcp_time_stamp - start_ts) > icsk->icsk_user_timeout)
+ goto abort;
+ max_probes = sysctl_tcp_retries2;
if (sock_flag(sk, SOCK_DEAD)) {
const int alive = inet_csk_rto_backoff(icsk, TCP_RTO_MAX) < TCP_RTO_MAX;
max_probes = tcp_orphan_retries(sk, alive);
-
+ if (!alive && icsk->icsk_backoff >= max_probes)
+ goto abort;
if (tcp_out_of_resources(sk, alive || icsk->icsk_probes_out <= max_probes))
return;
}
if (icsk->icsk_probes_out > max_probes) {
- tcp_write_err(sk);
+abort: tcp_write_err(sk);
} else {
/* Only send another probe if we didn't close things up. */
tcp_send_probe0(sk);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Andy Gospodarek @ 2014-09-23 4:11 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexei Starovoitov, Thomas Graf, Jiri Pirko, John Fastabend,
Jamal Hadi Salim, netdev@vger.kernel.org, David S. Miller,
Neil Horman, Andy Gospodarek, Daniel Borkmann, Or Gerlitz,
Jesse Gross, Pravin Shelar, Andy Zhou, Ben Hutchings,
Stephen Hemminger, Jeff Kirsher, Vladislav Yasevich, Cong Wang,
Eric Dumazet, Scott Feldman, Florian Fainelli, Roopa
In-Reply-To: <CA+mtBx-mJE6SW2bqf3_t6iu=o5FY1WAF1ByeZpDJnc+6fpK6nA@mail.gmail.com>
On Mon, Sep 22, 2014 at 07:16:47PM -0700, Tom Herbert wrote:
[...]
>
> Alexei, I believe you said previously said that SW should not dictate
> HW models. I agree with this, but also believe the converse is true--
> HW shouldn't dictate SW model. This is really why I'm raising the
> question of what it means to integrate a switch into the host stack.
Tom, when I read this I cannot help but remind myself that the
intentions/hopes/dreams of those on this thread and how different their
views can be on what it means to add additional 'offload support' to the
kernel.
There are clearly some that are most interested in how an eSwitch on an
SR-IOV capable NIC be controlled can provide traditional forwarding help
as well as offload the various technologies they hope to terminate
at/inside their endpoint (host/guest/container) -- Thomas's _simple_
use-case demonstrates this. ;) This is a logical extention/increase in
functionality that is offered in many eSwitches that was previously
hidden from the user with the first generation SR-IOV capable network
devices on hosts/servers.
Others (like Florian who has been working to extend DSA or those pushing
hardware vendors to make SDKs more open) where the existing bridging/
routing/offload code can take advantage of the hardware offload/encap
available in merchant silicon. The general idea seems to add the
knowledge of offload hardware to the kernel -- either via new ndo_ops or
netlink. This gives users who have this hardware the ability to have a
solution for their router/switch that makes it feel like Linux is
actually helping make forwarding decisions -- rather than just being the
kernel chosen to provide an environment where some other non-community
code runs that makes all of the decisions.
And now we also have the patchset that spawned what I think has been
more excellent discussion. Jiri and Scott's patches bring up another,
more generic model that while not currently backed by hardware provided
an example/vision for what could be done if such hardware existed and
how to consider interacting with that driver/hardware (that clearly has
been met with some resistance, but the discussion has been great).
There ultimate goals appear to be similar to those that want full
offload/fordwarding support for a device, but via a different method
than what some would consider standard.
I am personally hopeful that most who are passionate about this will be
able to get together next month at LPC (or send someone to represent
them!) so that those interested can sit in the same room and try to
better understand each others desires and start to form some concrete
direction towards a solution that seems to meet the needs of most while
not being an architectural disaster.
Of course that may be way too optimistic for this crowd! :-D
^ permalink raw reply
* Re: [PATCH] ath: change logging functions to return void
From: Kalle Valo @ 2014-09-23 4:20 UTC (permalink / raw)
To: Joe Perches
Cc: John W. Linville, Vladimir Kondratiev, ath10k, linux-wireless,
netdev, linux-kernel, wil6210, ath6kl
In-Reply-To: <1411407334.2952.46.camel@joe-AO725>
Joe Perches <joe@perches.com> writes:
> The return values are not used by callers of these functions
> so change the functions to return void.
>
> Other miscellanea:
>
> o add __printf verification to wil6210 logging functions
> No format/argument mismatches found
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> This change is associated to a desire to eventually
> change printk to return void.
>
> drivers/net/wireless/ath/ath10k/debug.c | 18 +++++-------------
> drivers/net/wireless/ath/ath10k/debug.h | 6 +++---
> drivers/net/wireless/ath/ath6kl/common.h | 2 +-
> drivers/net/wireless/ath/ath6kl/debug.c | 28 ++++++++--------------------
> drivers/net/wireless/ath/ath6kl/debug.h | 13 ++++++-------
For ath6kl and ath10k:
Acked-by: Kalle Valo <kvalo@qca.qualcomm.com>
> drivers/net/wireless/ath/wil6210/debug.c | 14 ++++----------
> drivers/net/wireless/ath/wil6210/wil6210.h | 7 +++++--
> 7 files changed, 32 insertions(+), 56 deletions(-)
John, as this patch also contains a wil6210 change how do you want to
handle this?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Mahesh Bandewar @ 2014-09-23 5:13 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <541EB184.3080705@redhat.com>
On Sun, Sep 21, 2014 at 4:07 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
> On 09/20/2014 10:04 PM, Mahesh Bandewar wrote:
>> On Sat, Sep 20, 2014 at 3:19 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>>> On 09/20/2014 02:09 AM, Mahesh Bandewar wrote:
>>>> On Fri, Sep 19, 2014 at 4:06 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>>>>>
>>>>> On 09/19/2014 12:00 PM, Nikolay Aleksandrov wrote:
>>>>>> On 09/18/2014 11:53 PM, Mahesh Bandewar wrote:
>>>>>>> Earlier change to use usable slave array for TLB mode had an additional
>>>>>>> performance advantage. So extending the same logic to all other modes
>>>>>>> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
>>>>>>> Also consolidating this with the earlier TLB change.
>>>>>>>
>>>>>>> The main idea is to build the usable slaves array in the control path
>>>>>>> and use that array for slave selection during xmit operation.
>>>>>>>
>>>>>>> Measured performance in a setup with a bond of 4x1G NICs with 200
>>>>>>> instances of netperf for the modes involved (3ad, xor, tlb)
>>>>>>> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>>>>>>>
>>>>>>> Mode TPS-Before TPS-After
>>>>>>>
>>>>>>> 802.3ad : 468,694 493,101
>>>>>>> TLB (lb=0): 392,583 392,965
>>>>>>> XOR : 475,696 484,517
>>>>>>>
>>>>>>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>>>>>> ---
>>>>>>> v1:
>>>>>>> (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
>>>>>>> the slave that need to be removed.
>>>>>>> (b) Freeing of array will assign NULL (to handle bond->down to bond->up
>>>>>>> transition gracefully.
>>>>>>> (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
>>>>>>> failure.
>>>>>>> (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
>>>>>>> will populate the array even if these parameters are not used.
>>>>>>> (e) 3AD: Should handle the ad_agg_selection_logic correctly.
>>>>>>> v2:
>>>>>>> (a) Removed rcu_read_{un}lock() calls from array manipulation code.
>>>>>>> (b) Slave link-events now refresh array for all these modes.
>>>>>>> (c) Moved free-array call from bond_close() to bond_uninit().
>>>>>>> v3:
>>>>>>> (a) Fixed null pointer dereference.
>>>>>>> (b) Removed bond->lock lockdep dependency.
>>>>>>> v4:
>>>>>>> (a) Made to changes to comply with Nikolay's locking changes
>>>>>>> (b) Added a work-queue to refresh slave-array when RTNL is not held
>>>>>>> (c) Array refresh happens ONLY with RTNL now.
>>>>>>> (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
>>>>>>>
>>>>> <<<snip>>>
>>>>>>> @@ -3839,6 +4003,7 @@ static void bond_uninit(struct net_device *bond_dev)
>>>>>>> struct bonding *bond = netdev_priv(bond_dev);
>>>>>>> struct list_head *iter;
>>>>>>> struct slave *slave;
>>>>>>> + struct bond_up_slave *arr;
>>>>>>>
>>>>>>> bond_netpoll_cleanup(bond_dev);
>>>>>>>
>>>>>>> @@ -3847,6 +4012,12 @@ static void bond_uninit(struct net_device *bond_dev)
>>>>>>> __bond_release_one(bond_dev, slave->dev, true);
>>>>>>> netdev_info(bond_dev, "Released all slaves\n");
>>>>>>>
>>>>> Sorry but I just spotted a major problem, bond_3ad_unbind_slave() (called
>>>>> from __bond_release_one) calls ad_agg_selection_logic() which can re-arm
>>>>> the slave_arr work after it's supposed to be stopped here (i.e. the bond
>>>>> device has been closed so all works should've been stopped) so we might
>>>>> leak memory and access freed memory after all since it'll keep
>>>>> re-scheduling itself until it can acquire rtnl which is after the bond
>>>>> device has been destroyed.
>>>>>
>>>> This should not be a problem. ndo_close (bond_close()) is called
>>>> before ndo_uninit(bond_uninit()), so the work-queues get cancelled
>>>> there so if rearm tries to schedule some work after queue gets
>>>> cancelled, it can't do much and wont harm anything.
>>>> Hence there wont be any arrays built once it's free-ed completely and
>>>> therefore no memory leak. I addded some instrumentation and tried
>>>> following sequence -
>>>>
>>>> # modprobe bonding mode=4
>>>> # ip link set bond0 up
>>>> # [Add ip]
>>>> # [Add default route]
>>>> # ifenslave bond0 eth0 eth1 eth2 eth3
>>>> ....
>>>> [Run some backgound traffic. I used netperf.]
>>>>
>>>> # ip link bond0 down
>>>>
>>>> I did not see anything "bad" happening. Did your trial produced
>>>> something unpleasant?
>>>>
>>> The test you've done is irrelevant to the situation that I described
>>> because ndo_uninit() is called when the device is being destroyed. Moreover
>>> the case I told you about would require to have an active aggregator and an
>>> inactive one (i.e. so agg selection logic will get called), here is the result:
>>> [ 428.916586] bond1 (unregistering): Removing an active aggregator
>>> [ 428.916589] Failed to build slave-array.
>>> [ 428.916849] bond1 (unregistering): Releasing active interface eth1
>>> [ 428.920342] bond1 (unregistering): Released all slaves
>>> [ 428.923043] Failed to update slave array from WT
>>> [ 428.924098] Failed to update slave array from WT
>>> [ 428.925125] Failed to update slave array from WT
>>> [ 428.926120] Failed to update slave array from WT
>>> [ 428.927096] Failed to update slave array from WT
>>> [ 428.928101] Failed to update slave array from WT
>>> [ 428.929120] Failed to update slave array from WT
>>> [ 428.930086] BUG: unable to handle kernel NULL pointer dereference at
>>> (null)
>>> [ 428.930644] IP: [<ffffffff810aa37b>] __queue_work+0x7b/0x350
>>> [ 428.930946] PGD 0
>>> [ 428.931053] Oops: 0000 [#1] SMP
>>> [ 428.931053] Modules linked in: sfc ptp pps_core mdio i2c_algo_bit mtd
>>> bonding(O) snd_hda_codec_generic joydev crct10dif_pclmul crc32_pclmul
>>> i2c_piix4 ppdev crc32c_intel ghash_clmulni_intel parport_pc snd_hda_intel
>>> snd_hda_controller snd_hda_codec snd_hwdep snd_pcm snd_timer 9pnet_virtio
>>> snd 9pnet pcspkr parport i2ccore serio_raw virtio_console virtio_balloon
>>> pvpanic soundcore virtio_blk virtio_net ata_generic floppy pata_acpi
>>> virtio_pci virtio_ring virtio
>>> [ 428.935022] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O
>>> 3.17.0-rc4+ #30
>>> [ 428.935022] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>>> [ 428.935022] task: ffffffff81c1b460 ti: ffffffff81c00000 task.ti:
>>> ffffffff81c00000
>>> [ 428.935022] RIP: 0010:[<ffffffff810aa37b>] [<ffffffff810aa37b>]
>>> __queue_work+0x7b/0x350
>>> [ 428.935022] RSP: 0018:ffff88005f003e28 EFLAGS: 00010086
>>> [ 428.935022] RAX: ffff88005c05c800 RBX: 0000000000000000 RCX:
>>> 0000000000000000
>>> [ 428.935022] RDX: 0000000000000000 RSI: 0000000000000006 RDI:
>>> ffff88005a4fbd58
>>> [ 428.935022] RBP: ffff88005f003e60 R08: 0000000000000046 R09:
>>> ffffffff8225abc2
>>> [ 428.935022] R10: 0000000000000004 R11: 0000000000000005 R12:
>>> ffff88005a4fbd58
>>> [ 428.935022] R13: 0000000000000008 R14: ffff88004b211800 R15:
>>> 00000000000102f0
>>> [ 428.935022] FS: 0000000000000000(0000) GS:ffff88005f000000(0000)
>>> knlGS:0000000000000000
>>> [ 428.935022] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [ 428.935022] CR2: 0000000000000000 CR3: 000000004abde000 CR4:
>>> 00000000000406f0
>>> [ 428.935022] Stack:
>>> [ 428.935022] 0a19522f72b12222 0000000081c1b460 ffffffff8225abc0
>>> ffff88005a4fbd78
>>> [ 428.935022] 0000000000000101 ffffffff810aa650 ffff88005a4fbd58
>>> ffff88005f003e70
>>> [ 428.935022] ffffffff810aa668 ffff88005f003ea8 ffffffff810f3536
>>> ffffffff8225abc0
>>> [ 428.935022] Call Trace:
>>> [ 428.935022] <IRQ>
>>> [ 428.935022]
>>> [ 428.935022] [<ffffffff810aa650>] ? __queue_work+0x350/0x350
>>> [ 428.935022] [<ffffffff810aa668>] delayed_work_timer_fn+0x18/0x20
>>> [ 428.935022] [<ffffffff810f3536>] call_timer_fn+0x36/0x120
>>> [ 428.935022] [<ffffffff810aa650>] ? __queue_work+0x350/0x350
>>> [ 428.935022] [<ffffffff810f38f5>] run_timer_softirq+0x1a5/0x320
>>> [ 428.935022] [<ffffffff81096dc5>] __do_softirq+0xf5/0x2b0
>>> [ 428.935022] [<ffffffff810971fd>] irq_exit+0xbd/0xd0
>>> [ 428.935022] [<ffffffff8173b715>] smp_apic_timer_interrupt+0x45/0x60
>>> [ 428.935022] [<ffffffff8173981d>] apic_timer_interrupt+0x6d/0x80
>>> [ 428.935022] <EOI>
>>> [ 428.935022]
>>> [ 428.935022] [<ffffffff810581c6>] ? native_safe_halt+0x6/0x10
>>> [ 428.935022] [<ffffffff8101f36f>] default_idle+0x1f/0xe0
>>> [ 428.935022] [<ffffffff8101fd8f>] arch_cpu_idle+0xf/0x20
>>> [ 428.935022] [<ffffffff810d25dd>] cpu_startup_entry+0x38d/0x3c0
>>> [ 428.935022] [<ffffffff81722927>] rest_init+0x87/0x90
>>> [ 428.935022] [<ffffffff81d3510e>] start_kernel+0x482/0x4a3
>>> [ 428.935022] [<ffffffff81d34a85>] ? set_init_arg+0x53/0x53
>>> [ 428.935022] [<ffffffff81d34120>] ? early_idt_handlers+0x120/0x120
>>> [ 428.935022] [<ffffffff81d345ee>] x86_64_start_reservations+0x2a/0x2c
>>> [ 428.935022] [<ffffffff81d3473d>] x86_64_start_kernel+0x14d/0x170
>>> [ 428.935022] Code: 84 bb 01 00 00 a8 02 0f 85 eb 00 00 00 48 63 45 d4 49
>>> 8b 9e 08 01 00 00 48 03 1c c5 60 fa d0 81 4c 89 e7 e8 18 f5 ff ff 48 85 c0
>>> <48> 8b 3b 0f 84 7c 01 00 00 48 39 c7 0f 84 73 01 00 00 48 89 c7
>>> [ 428.935022] RIP [<ffffffff810aa37b>] __queue_work+0x7b/0x350
>>> [ 428.935022] RSP <ffff88005f003e28>
>>> [ 428.935022] CR2: 0000000000000000
>>>
>>> This is because it keeps trying to re-schedule even though the interface's
>>> memory has been freed.
>>>
>> Hmm, how do we handle this?
>>
> This is tricky and what concerns me more is that people might make this
> mistake again in the future. It's easy to unknowingly make use of a
> function that re-schedules this from the wrong place.
> What I just noticed is that for all 3ad cases you could pull the scheduling
> in the bond_3ad_state_machine_handler() function.
> The call sites of ad_agg_selection_logic() are:
> - 3ad unbind slave (no need to schedule here as __bond_release_one would
> rebuild the array anyhow)
> - bond_3ad_state_machine_handler() <- here's where the schedule should
> happen as this gets stopped first when the bond is closed and can't get
> restarted unless it's opened again.
> - ad_port_selection_logic() <- this is called from
> bond_3ad_state_machine_handler() only, so this case will be handled as well.
>
> The other 2 functions that you convert - ad_enable/disable_collecting are
> used only from ad_mux_machine() which is only called in
> bond_3ad_state_machine_handler().
>
> So basically you can pull all rebuild schedules in their common caller -
> bond_3ad_state_machine_handler(), just make a flag to note that a rebuild
> is needed probably something similar to should_notify_rtnl.
> This way you can remove the scheduling from the various 3ad functions that
> may get used and will have it only in 1 place which is more easily controlled.
>
Well, I was just trying to avoid using flags to pass state from one to
another function so that we can update the array at one place. This
might introduce some bug so I was keeping it simple and build it only
when the condition requires it to build it. However I do not see how
this will fix the issue that you have seen, or would it? If so how?
> Of course, the alternative would be once again - convert
> bond_3ad_state_machine_handler() to RTNL, but that has its own set of problems.
>
It's convoluted, let's keep it simple for now :)
>>> While testing this I spotted another issue as well - Failed to build
>>> slave_arr message has been printed too many times because you print it in
>>> 3ad mode when there's no active aggregator (bond_3ad_get_active_agg_info
>>> check in bond_update_slave_arr) which leads to re-scheduling which also
>>> lead to a deadlock.
>>>
>> I think this can be corrected with pr_ratelimited() call.
>>
> IMO it shouldn't print anything if it couldn't rebuild the array due to
> missing active aggregator as that's not an error condition. It should
> though probably clean out the slave array because transmission shouldn't be
> possible without an active aggregator in 3ad.
>
Sure missing active aggregator is not an error but free-ing the slave
array silently would be bad either. At least we would see something in
the messages about "something" went wrong.
>
>
>
^ permalink raw reply
* 3.17 kernel crash while loading IPoIB
From: Sharma, Karun @ 2014-09-23 5:15 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Hello:
I am facing an issue wherein kernel 3.17 crashes while loading IPoIB module. I guess the issue discussed in this thread (https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg20963.html) is similar.
We were able to reproduce the issue with RC6 also. Here are the steps I followed:
I compiled and installed 3.17 kernel on top of RHEL 6.5.
Then I changed rdma.conf to not load IPoIB (If I don't do this, the kernel crashes while booting and starting RDMA service.)
After the server comes up, I just did "modprobe ib_ipoib" and kernel crashes.
Please see below the kernel back trace.
Seeing the announcement, it looks like RC6 will be the last RC for 3.17 kernel. Will the release happen with this issue? Is there any workaround available for this issue?
I am not sure what mechanism/process is used to report issue to kernel community.
Regards
Karun
Kernel Stack back-trace:
--------------------------------------------------------------------------------
crash> bt
PID: 145 TASK: ffff88081a580d90 CPU: 3 COMMAND: "kworker/3:1"
#0 [ffff88081a587750] machine_kexec at ffffffff8103c5d9
#1 [ffff88081a5877a0] crash_kexec at ffffffff810d0ff8
#2 [ffff88081a587870] oops_end at ffffffff81007570
#3 [ffff88081a5878a0] no_context at ffffffff81046e5e
#4 [ffff88081a5878f0] __bad_area_nosemaphore at ffffffff8104704d
#5 [ffff88081a587940] bad_area_nosemaphore at ffffffff81047163
#6 [ffff88081a587950] __do_page_fault at ffffffff81047722
#7 [ffff88081a587a70] do_page_fault at ffffffff8104798c
#8 [ffff88081a587a80] page_fault at ffffffff815aad62
[exception RIP: __dev_queue_xmit+894]
RIP: ffffffff814e17be RSP: ffff88081a587b38 RFLAGS: 00010282
RAX: ffff88087c1679fe RBX: ffff880812cc2500 RCX: 0000000000000044
RDX: 0000000000000008 RSI: 0000000000000000 RDI: ffff88081a363a9c
RBP: ffff88081a587b78 R8: 0000000000000000 R9: 0000000000000040
R10: 0000000000000000 R11: 000000007c1679ff R12: ffff88081a363a00
R13: ffff880814f3e000 R14: ffff880809535600 R15: 0000000000000000
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#9 [ffff88081a587b30] __dev_queue_xmit at ffffffff814e158b
#10 [ffff88081a587b80] dev_queue_xmit at ffffffff814e1930
#11 [ffff88081a587b90] neigh_connected_output at ffffffff814e81e8
#12 [ffff88081a587be0] ip6_finish_output2 at ffffffffa05ff8dd [ipv6]
#13 [ffff88081a587c40] ip6_finish_output at ffffffffa05ffe5f [ipv6]
#14 [ffff88081a587c60] ip6_output at ffffffffa05fff18 [ipv6]
#15 [ffff88081a587c90] ndisc_send_skb at ffffffffa06169a9 [ipv6]
#16 [ffff88081a587d40] ndisc_send_ns at ffffffffa0616bf6 [ipv6]
#17 [ffff88081a587db0] addrconf_dad_work at ffffffffa06076cb [ipv6]
#18 [ffff88081a587df0] process_one_work at ffffffff8106b23e
#19 [ffff88081a587e40] worker_thread at ffffffff8106b63f
#20 [ffff88081a587ec0] kthread at ffffffff8107041e
#21 [ffff88081a587f50] ret_from_fork at ffffffff815a92ac
---------------------------------------------------------------------------------
Regards,
Karun Sharma
^ permalink raw reply
* Re: [PATCH net-next] mellanox: Change en_print to return void
From: Amir Vadai @ 2014-09-23 5:45 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
In-Reply-To: <1411407622.2952.49.camel@joe-AO725>
On 9/22/2014 8:40 PM, Joe Perches wrote:
> No caller or macro uses the return value so make it void.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> This change is associated to a desire to eventually
> change printk to return void.
>
> drivers/net/ethernet/mellanox/mlx4/en_main.c | 17 +++++++----------
> drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 4 ++--
> 2 files changed, 9 insertions(+), 12 deletions(-)
Thanks Joe.
Acked-By: Amir Vadai <amirv@mellanox.com>
^ permalink raw reply
* [PATCHv3 1/1] bluetooth: Check for SCO type before setting retransmission effort
From: Bernhard Thaler @ 2014-09-23 5:59 UTC (permalink / raw)
To: marcel, gustavo, johan.hedberg, davem
Cc: linux-bluetooth, netdev, linux-kernel, Bernhard Thaler
In-Reply-To: <1411035321-10215-1-git-send-email-bernhard.thaler@r-it.at>
SCO connection cannot be setup to devices that do not support retransmission.
Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
adapted for this kernel version.
Code changed to check SCO/eSCO type before setting retransmission effort
and max. latency. The purpose of the patch is to support older devices not
capable of eSCO.
Tested on Blackberry 655+ headset which does not support retransmission.
Credits go to Alexander Sommerhuber.
Signed-off-by: Bernhard Thaler <bernhard.thaler@r-it.at>
---
net/bluetooth/hci_conn.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index faff624..32dcde1 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -38,7 +38,7 @@ struct sco_param {
u16 max_latency;
};
-static const struct sco_param sco_param_cvsd[] = {
+static const struct sco_param esco_param_cvsd[] = {
{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x000a }, /* S3 */
{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x0007 }, /* S2 */
{ EDR_ESCO_MASK | ESCO_EV3, 0x0007 }, /* S1 */
@@ -46,6 +46,11 @@ static const struct sco_param sco_param_cvsd[] = {
{ EDR_ESCO_MASK | ESCO_HV1, 0xffff }, /* D0 */
};
+static const struct sco_param sco_param_cvsd[] = {
+ { EDR_ESCO_MASK | ESCO_HV3, 0xffff }, /* D1 */
+ { EDR_ESCO_MASK | ESCO_HV1, 0xffff }, /* D0 */
+};
+
static const struct sco_param sco_param_wideband[] = {
{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x000d }, /* T2 */
{ EDR_ESCO_MASK | ESCO_EV3, 0x0008 }, /* T1 */
@@ -194,10 +199,17 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
param = &sco_param_wideband[conn->attempt - 1];
break;
case SCO_AIRMODE_CVSD:
- if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
- return false;
- cp.retrans_effort = 0x01;
- param = &sco_param_cvsd[conn->attempt - 1];
+ if (!lmp_esco_capable(conn->link)) {
+ if ((conn->attempt) > ARRAY_SIZE(sco_param_cvsd))
+ return false;
+ cp.retrans_effort = 0xff;
+ param = &sco_param_cvsd[conn->attempt - 1];
+ } else {
+ if (conn->attempt > ARRAY_SIZE(esco_param_cvsd))
+ return false;
+ cp.retrans_effort = 0x01;
+ param = &esco_param_cvsd[conn->attempt - 1];
+ }
break;
default:
return false;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected
From: Giuseppe CAVALLARO @ 2014-09-23 6:10 UTC (permalink / raw)
To: Kweh, Hock Leong, David Miller
Cc: rayagond@vayavyalabs.com, vbridgers2013@gmail.com, wens@csie.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Ong, Boon Leong, tobias.johannes.klausmann@mni.thm.de
In-Reply-To: <F54AEECA5E2B9541821D670476DAE19C2B7EAE19@PGSMSX102.gar.corp.intel.com>
On 9/23/2014 3:16 AM, Kweh, Hock Leong wrote:
>> -----Original Message-----
>> From: David Miller [mailto:davem@davemloft.net]
>> Sent: Tuesday, September 23, 2014 2:19 AM
>> From: Kweh Hock Leong <hock.leong.kweh@intel.com>
>> Date: Thu, 18 Sep 2014 20:34:10 +0800
>>
>> Giuseppe, Kweh, where are we with this patch?
>
> We are discussing whether this is the correct fix to the issue. Below is the discussion log:
>
>>> Hmm I am not sure this is the right fix. The driver has to fail if the
>>> main clock is not found. Indeed dev_warn has to be changed in dev_err.
>>>
>>> Take a look at Documentation/networking/stmmac.txt but I will post
>>> some patch to improve the documentation adding further detail for clocks too.
>>>
>>> The the logic behind the code is that the CSR clock will be set at
>>> runtime if in case of priv->plat->clk_csr ==0 or it will be forced to
>>> a fixed value if passed from the platform instead of.
>>> IIRC This was required on some platforms time ago.
>>> For sure the driver is designed to fail in case of no main clock is found.
>>>
>>> Peppe
>>
>> Hi Peppe,
>>
>> I understand your point from the code below (at file stmmac_main.c line 2784):
>>
>> /* If a specific clk_csr value is passed from the platform
>> * this means that the CSR Clock Range selection cannot be
>> * changed at run-time and it is fixed. Viceversa the driver'll try to
>> * set the MDC clock dynamically according to the csr actual
>> * clock input.
>> */
>> if (!priv->plat->clk_csr)
>> stmmac_clk_csr_set(priv);
>> else
>> priv->clk_csr = priv->plat->clk_csr;
>>
>>
>> I did search through the whole stmmac_main.c file and found that only stmmac_clk_csr_set()
>> function is leveraging the priv->stmmac_clk params for it calculation. By the logic point of view,
>> I do not need priv->stmmac_clk when I got priv->plat->clk_csr. With this thinking, I propose this
>> fix as when the probe get priv->plat->clk_csr, it shouldn't fail if priv->stmmac_clk has the error value.
>
> Hi Peppe,
>
> Are you trying to tell that if there is a fix CSR clock value, but driver cannot obtain the clk from devm_clk_get()
> (even it is not in use), the driver should fail the probe?
>
> You have a case with this condition:
> HW allow SW to discover the STMMAC controller but HW/SW configures not to supply the CLOCK to STMMAC controller?
>
> My understanding to these priv->plat->clk_csr and priv->stmmac_clk is that 1st one is fixed and 2nd one is dynamic.
> When fixed value occurs, dynamic one would not be use. Do I understand this correctly?
the logic is: the priv->stmmac_clk must be always provided from the
platform then we have two cases:
1) if priv->plat->clk_csr is also passed then it will be adopt in the
mdio functions to program the Reg4[5:2]
This was required in the past IIRC on SPEAr platforms.
2) if priv->plat->clk_csr is not passed from the platform then the
priv->clk_csr will be set according to the priv->stmmac_clk
and always used in the mdio part.
So IIUC now you are asking for not passing the priv->stmmac_clk
and warning this event w/o failing. Why you cannot pass this clock?
peppe
>
> Thanks.
>
>
> Regards,
> Wilson
>
^ permalink raw reply
* Re: net: stmmac glue layer for Amlogic Meson SoCs
From: Giuseppe CAVALLARO @ 2014-09-23 6:16 UTC (permalink / raw)
To: Beniamino Galvani, netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Carlo Caione, Jerry Cao, Victor Wan,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1411219757-9201-1-git-send-email-b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 9/20/2014 3:29 PM, Beniamino Galvani wrote:
> Hi,
>
> the Ethernet controller available in Amlogic Meson6 and Meson8 SoCs is
> a Synopsys DesignWare MAC IP core, already supported by the stmmac
> driver.
>
> These patches add a glue layer to the driver for the platform-specific
> settings required by the Amlogic variant.
>
> This has been tested on a Amlogic S802 device with the initial Meson
> support submitted by Carlo Caione [1].
patches look fine just a warning with checkpatch as shown below then
Acked-by: Giuseppe Cavallaro <peppe.cavallaro-qxv4g6HH51o@public.gmane.org>
[cavagiu@lxmcdt5 net.git]$ git-format-patch -2
0001-net-stmmac-add-Amlogic-Meson-glue-layer.patch
0002-net-stmmac-meson-document-device-tree-bindings.patch
[cavagiu@lxmcdt5 net.git]$ !./
./scripts/checkpatch.pl 000*
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57:
new file mode 100644
WARNING: DT compatible string vendor "amlogic" appears un-documented --
check ./Documentation/devicetree/bindings/vendor-prefixes.txt
#152: FILE: drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:34:
+ { .compatible = "amlogic,meson6-dwmac", .data = &meson6_dwmac_data},
total: 0 errors, 2 warnings, 0 checks, 108 lines checked
0001-net-stmmac-add-Amlogic-Meson-glue-layer.patch has style problems,
please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17:
new file mode 100644
total: 0 errors, 1 warnings, 25 lines checked
0002-net-stmmac-meson-document-device-tree-bindings.patch has style
problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> [1] http://lwn.net/Articles/612000/
>
> Beniamino Galvani (2):
> net: stmmac: add Amlogic Meson glue layer
> net: stmmac: meson: document device tree bindings
>
> .../devicetree/bindings/net/meson-dwmac.txt | 25 ++++++++
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 10 ++++
> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 67 ++++++++++++++++++++++
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 +
> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 +
> 6 files changed, 109 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/meson-dwmac.txt
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected
From: Kweh, Hock Leong @ 2014-09-23 7:03 UTC (permalink / raw)
To: Giuseppe CAVALLARO, David Miller
Cc: rayagond@vayavyalabs.com, vbridgers2013@gmail.com, wens@csie.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Ong, Boon Leong, tobias.johannes.klausmann@mni.thm.de
In-Reply-To: <54210EC1.9070507@st.com>
> -----Original Message-----
> From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@st.com]
> Sent: Tuesday, September 23, 2014 2:10 PM
>
> the logic is: the priv->stmmac_clk must be always provided from the platform
> then we have two cases:
>
> 1) if priv->plat->clk_csr is also passed then it will be adopt in the
> mdio functions to program the Reg4[5:2]
> This was required in the past IIRC on SPEAr platforms.
>
> 2) if priv->plat->clk_csr is not passed from the platform then the
> priv->clk_csr will be set according to the priv->stmmac_clk
> and always used in the mdio part.
>
> So IIUC now you are asking for not passing the priv->stmmac_clk and warning
> this event w/o failing. Why you cannot pass this clock?
>
> peppe
Hi peppe,
Appreciate for the explanation. Just to clarify that I am not asking not to pass in the priv->stmmac_clk.
In fact, the fix will fail at case 2 if driver cannot obtain the priv->stmmac_clk, but just not the case 1.
For case 1, seem like it does not require the stmmac_clk then I think it should be OK not to fail it when
driver did not get stmmac_clk but have the clk_csr set.
Anyway, I can change the fix by adding the clock registration APIs being call at the stmmac_pci.c probe there before
calling stmmac_dvr_probe. By doing this, it created a dependency to the pci driver that must have CONFIG_HAVE_CLK
to be turned on. Besides, I would need you guys to provide me information on other platforms about what is the best
value to set? Can I just set to zero since the stmmac_pci driver is always using the priv->plat->clk_csr?
Regards,
Wilson
^ permalink raw reply
* [PATCH net-next 1/3] ipv6: mld: rename mc_maxdelay into mc_uri
From: Daniel Borkmann @ 2014-09-23 7:03 UTC (permalink / raw)
To: davem; +Cc: hannes, netdev
In-Reply-To: <1411455828-5196-1-git-send-email-dborkman@redhat.com>
The name mc_maxdelay is quite confusing as it actually denotes the
unsolicited report interval. Since we have query response interval
named as mc_qri, name unsolicited report interval analogously as
mc_uri. Note that both are not the same!
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
include/net/if_inet6.h | 2 +-
net/ipv6/mcast.c | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index d07b1a6..8daf683 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -180,7 +180,7 @@ struct inet6_dev {
unsigned long mc_v1_seen; /* Max time we stay in MLDv1 mode */
unsigned long mc_qi; /* Query Interval */
unsigned long mc_qri; /* Query Response Interval */
- unsigned long mc_maxdelay;
+ unsigned long mc_uri; /* Unsolicited Report Interval */
struct timer_list mc_gq_timer; /* general query timer */
struct timer_list mc_ifc_timer; /* interface change timer */
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 592eba6..3d0e8fc 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -996,7 +996,7 @@ bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
static void mld_gq_start_timer(struct inet6_dev *idev)
{
- unsigned long tv = prandom_u32() % idev->mc_maxdelay;
+ unsigned long tv = prandom_u32() % idev->mc_uri;
idev->mc_gq_running = 1;
if (!mod_timer(&idev->mc_gq_timer, jiffies+tv+2))
@@ -1274,7 +1274,7 @@ static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
mld_update_qi(idev, mld);
mld_update_qri(idev, mld);
- idev->mc_maxdelay = *max_delay;
+ idev->mc_uri = *max_delay;
return 0;
}
@@ -2037,7 +2037,7 @@ void ipv6_mc_dad_complete(struct inet6_dev *idev)
mld_send_initial_cr(idev);
idev->mc_dad_count--;
if (idev->mc_dad_count)
- mld_dad_start_timer(idev, idev->mc_maxdelay);
+ mld_dad_start_timer(idev, idev->mc_uri);
}
}
@@ -2049,7 +2049,7 @@ static void mld_dad_timer_expire(unsigned long data)
if (idev->mc_dad_count) {
idev->mc_dad_count--;
if (idev->mc_dad_count)
- mld_dad_start_timer(idev, idev->mc_maxdelay);
+ mld_dad_start_timer(idev, idev->mc_uri);
}
in6_dev_put(idev);
}
@@ -2407,7 +2407,7 @@ static void mld_ifc_timer_expire(unsigned long data)
if (idev->mc_ifc_count) {
idev->mc_ifc_count--;
if (idev->mc_ifc_count)
- mld_ifc_start_timer(idev, idev->mc_maxdelay);
+ mld_ifc_start_timer(idev, idev->mc_uri);
}
in6_dev_put(idev);
}
@@ -2481,8 +2481,8 @@ static void ipv6_mc_reset(struct inet6_dev *idev)
idev->mc_qrv = sysctl_mld_qrv;
idev->mc_qi = MLD_QI_DEFAULT;
idev->mc_qri = MLD_QRI_DEFAULT;
+ idev->mc_uri = unsolicited_report_interval(idev);
idev->mc_v1_seen = 0;
- idev->mc_maxdelay = unsolicited_report_interval(idev);
}
/* Device going up */
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 2/3] ipv6: mld: do not overwrite uri when receiving an mldv2 query
From: Daniel Borkmann @ 2014-09-23 7:03 UTC (permalink / raw)
To: davem; +Cc: hannes, netdev
In-Reply-To: <1411455828-5196-1-git-send-email-dborkman@redhat.com>
While reviewing the code, I found it confusing why we update the URI when
receiving an MLDv2 query. The RFC does not mention any of this, and I also
double-checked with other implementations.
It is true that we start the general query timer with the received max_delay,
as mentioned in the older RFC2710, section 5.:
[...] "start timer" for the address on the interface, using a delay
value chosen uniformly from the interval [0, Maximum Response Delay],
where Maximum Response Delay is specified in the Query. If this is
an unsolicited Report, the timer is set to a delay value chosen
uniformly from the interval [0, [Unsolicited Report Interval] ].
It however does not say anywhere that we are supposed to overwrite that
value. The purpose of the report is quite different and described as:
When a node starts listening to a multicast address on an interface,
it should immediately transmit an unsolicited Report for that address
on that interface, in case it is the first listener on the link.
To cover the possibility of the initial Report being lost or damaged,
it is recommended that it be repeated once or twice after short delays
[Unsolicited Report Interval]. (A simple way to accomplish this is to
send the initial Report and then act as if a Multicast-Address-Specific
Query was received for that address, and set a timer appropriately).
RFC3810, section 9.11. only changed that default interval into 1 second (in
contrast to the older RFC2710). Therefore, do not update the URI sysctl
provided interval value when receiving an MLDv2 query, only pass that max
delay as mentioned in section 5. along to mld_gq_start_timer(). This
behaviour seems to be the case since the initial implementation of MLDv2.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
[ Sending to net-next to let this linger a bit here first, seems to be
the case like this since initial MLDv2. ]
net/ipv6/mcast.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 3d0e8fc..2a4d2b1 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -994,9 +994,9 @@ bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
return rv;
}
-static void mld_gq_start_timer(struct inet6_dev *idev)
+static void mld_gq_start_timer(struct inet6_dev *idev, unsigned long delay)
{
- unsigned long tv = prandom_u32() % idev->mc_uri;
+ unsigned long tv = prandom_u32() % delay;
idev->mc_gq_running = 1;
if (!mod_timer(&idev->mc_gq_timer, jiffies+tv+2))
@@ -1274,8 +1274,6 @@ static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
mld_update_qi(idev, mld);
mld_update_qri(idev, mld);
- idev->mc_uri = *max_delay;
-
return 0;
}
@@ -1345,7 +1343,7 @@ int igmp6_event_query(struct sk_buff *skb)
if (mlh2->mld2q_nsrcs)
return -EINVAL; /* no sources allowed */
- mld_gq_start_timer(idev);
+ mld_gq_start_timer(idev, max_delay);
return 0;
}
/* mark sources to include, if group & source-specific */
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 0/3] mld updates, part 2
From: Daniel Borkmann @ 2014-09-23 7:03 UTC (permalink / raw)
To: davem; +Cc: hannes, netdev
Daniel Borkmann (3):
ipv6: mld: rename mc_maxdelay into mc_uri
ipv6: mld: do not overwrite uri when receiving an mldv2 query
ipv6: mld: remove duplicate code from mld_update_qri
include/net/if_inet6.h | 2 +-
net/ipv6/mcast.c | 31 ++++++++++++-------------------
2 files changed, 13 insertions(+), 20 deletions(-)
--
1.7.11.7
^ 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