* [PATCH v2] cfg80211 debugfs: Cleanup some checkpatch issues
From: Dmitriy Pichugin @ 2017-01-28 14:06 UTC (permalink / raw)
To: johannes, davem; +Cc: linux-wireless, netdev, linux-kernel
This fixes the checkpatch.pl warnings:
* Macros should not use a trailing semicolon.
* Spaces required around that '='.
* Symbolic permissions 'S_IRUGO' are not preferred.
Signed-off-by: Dmitriy Pichugin <smokeman85@gmail.com>
---
net/wireless/debugfs.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/wireless/debugfs.c b/net/wireless/debugfs.c
index 5d45391..30fc6eb 100644
--- a/net/wireless/debugfs.c
+++ b/net/wireless/debugfs.c
@@ -17,7 +17,7 @@
static ssize_t name## _read(struct file *file, char __user *userbuf, \
size_t count, loff_t *ppos) \
{ \
- struct wiphy *wiphy= file->private_data; \
+ struct wiphy *wiphy = file->private_data; \
char buf[buflen]; \
int res; \
\
@@ -29,14 +29,14 @@
.read = name## _read, \
.open = simple_open, \
.llseek = generic_file_llseek, \
-};
+}
DEBUGFS_READONLY_FILE(rts_threshold, 20, "%d",
- wiphy->rts_threshold)
+ wiphy->rts_threshold);
DEBUGFS_READONLY_FILE(fragmentation_threshold, 20, "%d",
wiphy->frag_threshold);
DEBUGFS_READONLY_FILE(short_retry_limit, 20, "%d",
- wiphy->retry_short)
+ wiphy->retry_short);
DEBUGFS_READONLY_FILE(long_retry_limit, 20, "%d",
wiphy->retry_long);
@@ -103,7 +103,7 @@ static ssize_t ht40allow_map_read(struct file *file,
};
#define DEBUGFS_ADD(name) \
- debugfs_create_file(#name, S_IRUGO, phyd, &rdev->wiphy, &name## _ops);
+ debugfs_create_file(#name, 0444, phyd, &rdev->wiphy, &name## _ops)
void cfg80211_debugfs_rdev_add(struct cfg80211_registered_device *rdev)
{
--
1.9.1
^ permalink raw reply related
* [PATCH net-next] net: add devm version of alloc_etherdev_mqs function
From: Rafał Miłecki @ 2017-01-28 14:15 UTC (permalink / raw)
To: David S . Miller
Cc: Pablo Neira Ayuso, Andrew Lunn, Joe Perches, Tom Herbert,
Alexander Duyck, Jesper Dangaard Brouer, Jarod Wilson,
Sabrina Dubroca, netdev, linux-kernel, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This patch adds devm_alloc_etherdev_mqs function and devm_alloc_etherdev
macro. These can be used for simpler netdev allocation without having to
care about calling free_netdev.
Thanks to this change drivers, their error paths and removal paths may
get simpler by a bit.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
I'm working on V2 of:
[PATCH 0/3] net-next: use one struct bgmac & add PHY support
and I just realized I could get a way simpler error path in bgmac with a
devm_alloc_etherdev helper. So there is my suggestion for adding it.
---
include/linux/etherdevice.h | 5 +++++
net/ethernet/eth.c | 28 ++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 42add77ae47d..c62b709b1ce0 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -54,6 +54,11 @@ struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
#define alloc_etherdev(sizeof_priv) alloc_etherdev_mq(sizeof_priv, 1)
#define alloc_etherdev_mq(sizeof_priv, count) alloc_etherdev_mqs(sizeof_priv, count, count)
+struct net_device *devm_alloc_etherdev_mqs(struct device *dev, int sizeof_priv,
+ unsigned int txqs,
+ unsigned int rxqs);
+#define devm_alloc_etherdev(dev, sizeof_priv) devm_alloc_etherdev_mqs(dev, sizeof_priv, 1, 1)
+
struct sk_buff **eth_gro_receive(struct sk_buff **head,
struct sk_buff *skb);
int eth_gro_complete(struct sk_buff *skb, int nhoff);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 8c5a479681ca..efdaaab735fc 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -392,6 +392,34 @@ struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
}
EXPORT_SYMBOL(alloc_etherdev_mqs);
+static void devm_free_netdev(struct device *dev, void *res)
+{
+ free_netdev(*(struct net_device **)res);
+}
+
+struct net_device *devm_alloc_etherdev_mqs(struct device *dev, int sizeof_priv,
+ unsigned int txqs, unsigned int rxqs)
+{
+ struct net_device **dr;
+ struct net_device *netdev;
+
+ dr = devres_alloc(devm_free_netdev, sizeof(*dr), GFP_KERNEL);
+ if (!dr)
+ return NULL;
+
+ netdev = alloc_etherdev_mqs(sizeof_priv, txqs, rxqs);
+ if (!netdev) {
+ devres_free(dr);
+ return NULL;
+ }
+
+ *dr = netdev;
+ devres_add(dev, dr);
+
+ return netdev;
+}
+EXPORT_SYMBOL(devm_alloc_etherdev_mqs);
+
ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len)
{
return scnprintf(buf, PAGE_SIZE, "%*phC\n", len, addr);
--
2.11.0
^ permalink raw reply related
* [PATCHv2 RFC net-next 4/7] tcp: replace dst_confirm with sk_dst_confirm
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
In-Reply-To: <1485613578-19973-1-git-send-email-ja@ssi.bg>
When same struct dst_entry can be used for many different
neighbours we can not use it for pending confirmations.
Use the new sk_dst_confirm() helper to propagate the
indication from received packets to sock_confirm_neigh().
Reported-by: YueHaibing <yuehaibing@huawei.com>
Fixes: 5110effee8fd ("net: Do delayed neigh confirmation.")
Fixes: f2bb4bedf35d ("ipv4: Cache output routes in fib_info nexthops.")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
net/ipv4/tcp_input.c | 12 +++---------
net/ipv4/tcp_metrics.c | 7 ++-----
net/ipv4/tcp_output.c | 2 ++
3 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3de6eba..b3e88bb 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3644,11 +3644,8 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
if (tp->tlp_high_seq)
tcp_process_tlp_ack(sk, ack, flag);
- if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP)) {
- struct dst_entry *dst = __sk_dst_get(sk);
- if (dst)
- dst_confirm(dst);
- }
+ if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP))
+ sk_dst_confirm(sk);
if (icsk->icsk_pending == ICSK_TIME_RETRANS)
tcp_schedule_loss_probe(sk);
@@ -5995,7 +5992,6 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
break;
case TCP_FIN_WAIT1: {
- struct dst_entry *dst;
int tmo;
/* If we enter the TCP_FIN_WAIT1 state and we are a
@@ -6022,9 +6018,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
tcp_set_state(sk, TCP_FIN_WAIT2);
sk->sk_shutdown |= SEND_SHUTDOWN;
- dst = __sk_dst_get(sk);
- if (dst)
- dst_confirm(dst);
+ sk_dst_confirm(sk);
if (!sock_flag(sk, SOCK_DEAD)) {
/* Wake up lingering close() */
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index b9ed0d5..0f46e5f 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -375,12 +375,10 @@ void tcp_update_metrics(struct sock *sk)
u32 val;
int m;
+ sk_dst_confirm(sk);
if (sysctl_tcp_nometrics_save || !dst)
return;
- if (dst->flags & DST_HOST)
- dst_confirm(dst);
-
rcu_read_lock();
if (icsk->icsk_backoff || !tp->srtt_us) {
/* This session failed to estimate rtt. Why?
@@ -493,11 +491,10 @@ void tcp_init_metrics(struct sock *sk)
struct tcp_metrics_block *tm;
u32 val, crtt = 0; /* cached RTT scaled by 8 */
+ sk_dst_confirm(sk);
if (!dst)
goto reset;
- dst_confirm(dst);
-
rcu_read_lock();
tm = tcp_get_metrics(sk, dst, true);
if (!tm) {
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 671c695..c1f8a59 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -973,6 +973,8 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
skb_set_hash_from_sk(skb, sk);
atomic_add(skb->truesize, &sk->sk_wmem_alloc);
+ skb_set_dst_pending_confirm(skb, sk->sk_dst_pending_confirm);
+
/* Build TCP header and checksum it. */
th = (struct tcphdr *)skb->data;
th->source = inet->inet_sport;
--
1.9.3
^ permalink raw reply related
* [PATCHv2 RFC net-next 7/7] net: pending_confirm is not used anymore
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
In-Reply-To: <1485613578-19973-1-git-send-email-ja@ssi.bg>
When same struct dst_entry can be used for many different
neighbours we can not use it for pending confirmations.
As last step, we can remove the pending_confirm flag.
Reported-by: YueHaibing <yuehaibing@huawei.com>
Fixes: 5110effee8fd ("net: Do delayed neigh confirmation.")
Fixes: f2bb4bedf35d ("ipv4: Cache output routes in fib_info nexthops.")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/net/dst.h | 14 ++------------
net/core/dst.c | 1 -
2 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index 3a3b34b..84a1043 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -59,8 +59,6 @@ struct dst_entry {
#define DST_XFRM_QUEUE 0x0100
#define DST_METADATA 0x0200
- unsigned short pending_confirm;
-
short error;
/* A non-zero value of dst->obsolete forces by-hand validation
@@ -78,6 +76,8 @@ struct dst_entry {
#define DST_OBSOLETE_KILL -2
unsigned short header_len; /* more space at head required */
unsigned short trailer_len; /* space to reserve at tail */
+ unsigned short __pad3;
+
#ifdef CONFIG_IP_ROUTE_CLASSID
__u32 tclassid;
#else
@@ -440,7 +440,6 @@ static inline void dst_rcu_free(struct rcu_head *head)
static inline void dst_confirm(struct dst_entry *dst)
{
- dst->pending_confirm = 1;
}
static inline int dst_neigh_output(struct dst_entry *dst, struct neighbour *n,
@@ -448,15 +447,6 @@ static inline int dst_neigh_output(struct dst_entry *dst, struct neighbour *n,
{
const struct hh_cache *hh;
- if (dst->pending_confirm) {
- unsigned long now = jiffies;
-
- dst->pending_confirm = 0;
- /* avoid dirtying neighbour */
- if (n->confirmed != now)
- n->confirmed = now;
- }
-
hh = &n->hh;
if ((n->nud_state & NUD_CONNECTED) && hh->hh_len)
return neigh_hh_output(hh, skb);
diff --git a/net/core/dst.c b/net/core/dst.c
index b5cbbe0..960e503 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -190,7 +190,6 @@ void dst_init(struct dst_entry *dst, struct dst_ops *ops,
dst->__use = 0;
dst->lastuse = jiffies;
dst->flags = flags;
- dst->pending_confirm = 0;
dst->next = NULL;
if (!(flags & DST_NOCOUNT))
dst_entries_add(ops, 1);
--
1.9.3
^ permalink raw reply related
* [PATCHv2 RFC net-next 0/7] net: dst_confirm replacement
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
v1->v2:
- patch 1:
put sk_dst_pending_confirm in TX cacheline
- patch 2:
add skb_set_dst_pending_confirm and skb_get_dst_pending_confirm helpers
- patch 3:
use skb_set_dst_pending_confirm
remove check from sctp_transport_dst_confirm, directly assign
- patch 4:
use skb_set_dst_pending_confirm
- patch 6:
use skb_set_dst_pending_confirm
This patchset addresses the problem of neighbour
confirmation where received replies from one nexthop
can cause confirmation of different nexthop when using
the same dst. Thanks to YueHaibing <yuehaibing@huawei.com>
for tracking the dst->pending_confirm problem.
Sockets can obtain cached output route. Such
routes can be to known nexthop (rt_gateway=IP) or to be
used simultaneously for different nexthop IPs by different
subnet prefixes (nh->nh_scope = RT_SCOPE_HOST, rt_gateway=0).
At first look, there are more problems:
- dst_confirm() sets flag on dst and not on dst->path,
as result, indication is lost when XFRM is used
- DNAT can change the nexthop, so the really used nexthop is
not confirmed
So, the following solution is to avoid using
dst->pending_confirm.
The current dst_confirm() usage is as follows:
Protocols confirming dst on received packets:
- TCP (1 dst per socket)
- SCTP (1 dst per transport)
- CXGB*
Protocols supporting sendmsg with MSG_CONFIRM [ | MSG_PROBE ] to
confirm neighbour:
- UDP IPv4/IPv6
- ICMPv4 PING
- RAW IPv4/IPv6
- L2TP/IPv6
MSG_CONFIRM for other purposes (fix not needed):
- CAN
Sending without locking the socket:
- UDP (when no cork)
- RAW (when hdrincl=1)
Redirects from old to new GW:
- rt6_do_redirect
The patchset includes the following changes:
1. sock: add sk_dst_pending_confirm flag
- used only by TCP with patch 4 to remember the received
indication in sk->sk_dst_pending_confirm
2. net: add dst_pending_confirm flag to skbuff
- skb->dst_pending_confirm will be used by all protocols
in following patches, via skb_{set,get}_dst_pending_confirm
3. sctp: add dst_pending_confirm flag
- SCTP uses per-transport dsts and can not use
sk->sk_dst_pending_confirm like TCP
4. tcp: replace dst_confirm with sk_dst_confirm
5. net: add confirm_neigh method to dst_ops
- IPv4 and IPv6 provision for slow neigh lookups for MSG_PROBE users.
I decided to use neigh lookup only for this case because on
MSG_PROBE the skb may pass MTU checks but it does not reach
the neigh confirmation code. This patch will be used from patch 6.
- xfrm_confirm_neigh: support is incomplete here, only routes with
known nexthops (gateway) are supported because the tunnel address
is slow to obtain. Or there is solution to this problem?
6. net: use dst_confirm_neigh for UDP, RAW, ICMP, L2TP
- dst_confirm conversion for UDP, RAW, ICMP and L2TP/IPv6
- these protocols use MSG_CONFIRM propagated by ip*_append_data
to skb->dst_pending_confirm. sk->sk_dst_pending_confirm is not
used because some sending paths do not lock the socket. For
MSG_PROBE we use the slow lookup (dst_confirm_neigh).
- there are also 2 cases that need the slow lookup:
__ip6_rt_update_pmtu and rt6_do_redirect. I hope
&ipv6_hdr(skb)->saddr is the correct nexthop address to use here.
7. net: pending_confirm is not used anymore
- I failed to understand the CXGB* code, I see dst_confirm()
calls but I'm not sure dst_neigh_output() was called. For now
I just removed the dst->pending_confirm flag and left all
dst_confirm() calls there. Any better idea?
- Now may be old function neigh_output() should be restored
instead of dst_neigh_output?
Julian Anastasov (7):
sock: add sk_dst_pending_confirm flag
net: add dst_pending_confirm flag to skbuff
sctp: add dst_pending_confirm flag
tcp: replace dst_confirm with sk_dst_confirm
net: add confirm_neigh method to dst_ops
net: use dst_confirm_neigh for UDP, RAW, ICMP, L2TP
net: pending_confirm is not used anymore
drivers/net/vrf.c | 5 ++++-
include/linux/skbuff.h | 12 ++++++++++++
include/net/arp.h | 16 ++++++++++++++++
include/net/dst.h | 21 +++++++++------------
include/net/dst_ops.h | 2 ++
include/net/ndisc.h | 17 +++++++++++++++++
include/net/sctp/sctp.h | 6 ++----
include/net/sctp/structs.h | 4 ++++
include/net/sock.h | 26 ++++++++++++++++++++++++++
net/core/dst.c | 1 -
net/core/sock.c | 2 ++
net/ipv4/ip_output.c | 11 ++++++++++-
net/ipv4/ping.c | 3 ++-
net/ipv4/raw.c | 6 +++++-
net/ipv4/route.c | 19 +++++++++++++++++++
net/ipv4/tcp_input.c | 12 +++---------
net/ipv4/tcp_metrics.c | 7 ++-----
net/ipv4/tcp_output.c | 2 ++
net/ipv4/udp.c | 3 ++-
net/ipv6/ip6_output.c | 7 +++++++
net/ipv6/raw.c | 6 +++++-
net/ipv6/route.c | 43 ++++++++++++++++++++++++++++++-------------
net/ipv6/udp.c | 3 ++-
net/l2tp/l2tp_ip6.c | 3 ++-
net/sctp/associola.c | 3 +--
net/sctp/output.c | 10 +++++++++-
net/sctp/outqueue.c | 2 +-
net/sctp/sm_make_chunk.c | 6 ++----
net/sctp/sm_sideeffect.c | 2 +-
net/sctp/socket.c | 4 ++--
net/sctp/transport.c | 17 ++++++++++++++++-
net/xfrm/xfrm_policy.c | 16 ++++++++++++++++
32 files changed, 233 insertions(+), 64 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCHv2 RFC net-next 1/7] sock: add sk_dst_pending_confirm flag
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
In-Reply-To: <1485613578-19973-1-git-send-email-ja@ssi.bg>
Add new sock flag to allow sockets to confirm neighbour.
When same struct dst_entry can be used for many different
neighbours we can not use it for pending confirmations.
As not all call paths lock the socket use full word for
the flag.
Add sk_dst_confirm as replacement for dst_confirm when
called for received packets.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/net/sock.h | 12 ++++++++++++
net/core/sock.c | 2 ++
2 files changed, 14 insertions(+)
diff --git a/include/net/sock.h b/include/net/sock.h
index 7144750..e113786 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -240,6 +240,7 @@ struct sock_common {
* @sk_wq: sock wait queue and async head
* @sk_rx_dst: receive input route used by early demux
* @sk_dst_cache: destination cache
+ * @sk_dst_pending_confirm: need to confirm neighbour
* @sk_policy: flow policy
* @sk_receive_queue: incoming packets
* @sk_wmem_alloc: transmit queue bytes committed
@@ -393,6 +394,8 @@ struct sock {
struct sk_buff_head sk_write_queue;
__s32 sk_peek_off;
int sk_write_pending;
+ __u32 sk_dst_pending_confirm;
+ /* Note: 32bit hole on 64bit arches */
long sk_sndtimeo;
struct timer_list sk_timer;
__u32 sk_priority;
@@ -1764,6 +1767,7 @@ static inline void dst_negative_advice(struct sock *sk)
if (ndst != dst) {
rcu_assign_pointer(sk->sk_dst_cache, ndst);
sk_tx_queue_clear(sk);
+ sk->sk_dst_pending_confirm = 0;
}
}
}
@@ -1774,6 +1778,7 @@ static inline void dst_negative_advice(struct sock *sk)
struct dst_entry *old_dst;
sk_tx_queue_clear(sk);
+ sk->sk_dst_pending_confirm = 0;
/*
* This can be called while sk is owned by the caller only,
* with no state that can be checked in a rcu_dereference_check() cond
@@ -1789,6 +1794,7 @@ static inline void dst_negative_advice(struct sock *sk)
struct dst_entry *old_dst;
sk_tx_queue_clear(sk);
+ sk->sk_dst_pending_confirm = 0;
old_dst = xchg((__force struct dst_entry **)&sk->sk_dst_cache, dst);
dst_release(old_dst);
}
@@ -1809,6 +1815,12 @@ static inline void dst_negative_advice(struct sock *sk)
struct dst_entry *sk_dst_check(struct sock *sk, u32 cookie);
+static inline void sk_dst_confirm(struct sock *sk)
+{
+ if (!sk->sk_dst_pending_confirm)
+ sk->sk_dst_pending_confirm = 1;
+}
+
bool sk_mc_loop(struct sock *sk);
static inline bool sk_can_gso(const struct sock *sk)
diff --git a/net/core/sock.c b/net/core/sock.c
index 8b35debf..b743565 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -502,6 +502,7 @@ struct dst_entry *__sk_dst_check(struct sock *sk, u32 cookie)
if (dst && dst->obsolete && dst->ops->check(dst, cookie) == NULL) {
sk_tx_queue_clear(sk);
+ sk->sk_dst_pending_confirm = 0;
RCU_INIT_POINTER(sk->sk_dst_cache, NULL);
dst_release(dst);
return NULL;
@@ -1519,6 +1520,7 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
af_family_clock_key_strings[newsk->sk_family]);
newsk->sk_dst_cache = NULL;
+ newsk->sk_dst_pending_confirm = 0;
newsk->sk_wmem_queued = 0;
newsk->sk_forward_alloc = 0;
atomic_set(&newsk->sk_drops, 0);
--
1.9.3
^ permalink raw reply related
* [PATCHv2 RFC net-next 6/7] net: use dst_confirm_neigh for UDP, RAW, ICMP, L2TP
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
In-Reply-To: <1485613578-19973-1-git-send-email-ja@ssi.bg>
When same struct dst_entry can be used for many different
neighbours we can not use it for pending confirmations.
The datagram protocols can use MSG_CONFIRM to confirm the
neighbour. When used with MSG_PROBE we do not reach the
code where neighbour is confirmed, so we have to do the
same slow lookup by using the dst_confirm_neigh() helper.
When MSG_PROBE is not used, ip_append_data/ip6_append_data
will set the skb flag dst_pending_confirm.
Reported-by: YueHaibing <yuehaibing@huawei.com>
Fixes: 5110effee8fd ("net: Do delayed neigh confirmation.")
Fixes: f2bb4bedf35d ("ipv4: Cache output routes in fib_info nexthops.")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
net/ipv4/ip_output.c | 6 ++++++
net/ipv4/ping.c | 3 ++-
net/ipv4/raw.c | 6 +++++-
net/ipv4/udp.c | 3 ++-
net/ipv6/ip6_output.c | 6 ++++++
net/ipv6/raw.c | 6 +++++-
net/ipv6/route.c | 27 ++++++++++++++-------------
net/ipv6/udp.c | 3 ++-
net/l2tp/l2tp_ip6.c | 3 ++-
9 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 27f1db7..ff0fcaa 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -889,6 +889,9 @@ static inline int ip_ufo_append_data(struct sock *sk,
skb->csum = 0;
+ if (flags & MSG_CONFIRM)
+ skb_set_dst_pending_confirm(skb, 1);
+
__skb_queue_tail(queue, skb);
} else if (skb_is_gso(skb)) {
goto append;
@@ -1089,6 +1092,9 @@ static int __ip_append_data(struct sock *sk,
exthdrlen = 0;
csummode = CHECKSUM_NONE;
+ if ((flags & MSG_CONFIRM) && !skb_prev)
+ skb_set_dst_pending_confirm(skb, 1);
+
/*
* Put the packet on the pending queue.
*/
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 592db6a..6ee792d 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -848,7 +848,8 @@ static int ping_v4_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
return err;
do_confirm:
- dst_confirm(&rt->dst);
+ if (msg->msg_flags & MSG_PROBE)
+ dst_confirm_neigh(&rt->dst, &fl4.daddr);
if (!(msg->msg_flags & MSG_PROBE) || len)
goto back_from_confirm;
err = 0;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 4e49e5c..8119e1f 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -383,6 +383,9 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
sock_tx_timestamp(sk, sockc->tsflags, &skb_shinfo(skb)->tx_flags);
+ if (flags & MSG_CONFIRM)
+ skb_set_dst_pending_confirm(skb, 1);
+
skb->transport_header = skb->network_header;
err = -EFAULT;
if (memcpy_from_msg(iph, msg, length))
@@ -666,7 +669,8 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
return len;
do_confirm:
- dst_confirm(&rt->dst);
+ if (msg->msg_flags & MSG_PROBE)
+ dst_confirm_neigh(&rt->dst, &fl4.daddr);
if (!(msg->msg_flags & MSG_PROBE) || len)
goto back_from_confirm;
err = 0;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d6dddcf..4bdb358 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1088,7 +1088,8 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
return err;
do_confirm:
- dst_confirm(&rt->dst);
+ if (msg->msg_flags & MSG_PROBE)
+ dst_confirm_neigh(&rt->dst, &fl4->daddr);
if (!(msg->msg_flags&MSG_PROBE) || len)
goto back_from_confirm;
err = 0;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 7d90cab..5d944c1 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1145,6 +1145,9 @@ static inline int ip6_ufo_append_data(struct sock *sk,
skb->protocol = htons(ETH_P_IPV6);
skb->csum = 0;
+ if (flags & MSG_CONFIRM)
+ skb_set_dst_pending_confirm(skb, 1);
+
__skb_queue_tail(queue, skb);
} else if (skb_is_gso(skb)) {
goto append;
@@ -1517,6 +1520,9 @@ static int __ip6_append_data(struct sock *sk,
exthdrlen = 0;
dst_exthdrlen = 0;
+ if ((flags & MSG_CONFIRM) && !skb_prev)
+ skb_set_dst_pending_confirm(skb, 1);
+
/*
* Put the packet on the pending queue
*/
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index ea89073..f174e76 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -654,6 +654,9 @@ static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, int length,
skb->ip_summed = CHECKSUM_NONE;
+ if (flags & MSG_CONFIRM)
+ skb_set_dst_pending_confirm(skb, 1);
+
skb->transport_header = skb->network_header;
err = memcpy_from_msg(iph, msg, length);
if (err)
@@ -934,7 +937,8 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
txopt_put(opt_to_free);
return err < 0 ? err : len;
do_confirm:
- dst_confirm(dst);
+ if (msg->msg_flags & MSG_PROBE)
+ dst_confirm_neigh(dst, &fl6.daddr);
if (!(msg->msg_flags & MSG_PROBE) || len)
goto back_from_confirm;
err = 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c876940..15e45a6 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1375,6 +1375,7 @@ static bool rt6_cache_allowed_for_pmtu(const struct rt6_info *rt)
static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
const struct ipv6hdr *iph, u32 mtu)
{
+ const struct in6_addr *daddr, *saddr;
struct rt6_info *rt6 = (struct rt6_info *)dst;
if (rt6->rt6i_flags & RTF_LOCAL)
@@ -1383,26 +1384,26 @@ static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
if (dst_metric_locked(dst, RTAX_MTU))
return;
- dst_confirm(dst);
+ if (iph) {
+ daddr = &iph->daddr;
+ saddr = &iph->saddr;
+ } else if (sk) {
+ daddr = &sk->sk_v6_daddr;
+ saddr = &inet6_sk(sk)->saddr;
+ } else {
+ daddr = NULL;
+ saddr = NULL;
+ }
+ dst_confirm_neigh(dst, daddr);
mtu = max_t(u32, mtu, IPV6_MIN_MTU);
if (mtu >= dst_mtu(dst))
return;
if (!rt6_cache_allowed_for_pmtu(rt6)) {
rt6_do_update_pmtu(rt6, mtu);
- } else {
- const struct in6_addr *daddr, *saddr;
+ } else if (daddr) {
struct rt6_info *nrt6;
- if (iph) {
- daddr = &iph->daddr;
- saddr = &iph->saddr;
- } else if (sk) {
- daddr = &sk->sk_v6_daddr;
- saddr = &inet6_sk(sk)->saddr;
- } else {
- return;
- }
nrt6 = ip6_rt_cache_alloc(rt6, daddr, saddr);
if (nrt6) {
rt6_do_update_pmtu(nrt6, mtu);
@@ -2274,7 +2275,7 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
* Look, redirects are sent only in response to data packets,
* so that this nexthop apparently is reachable. --ANK
*/
- dst_confirm(&rt->dst);
+ dst_confirm_neigh(&rt->dst, &ipv6_hdr(skb)->saddr);
neigh = __neigh_lookup(&nd_tbl, &msg->target, skb->dev, 1);
if (!neigh)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 05d6932..9402f7a 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1295,7 +1295,8 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
return err;
do_confirm:
- dst_confirm(dst);
+ if (msg->msg_flags & MSG_PROBE)
+ dst_confirm_neigh(dst, &fl6.daddr);
if (!(msg->msg_flags&MSG_PROBE) || len)
goto back_from_confirm;
err = 0;
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 4b06eb4..734798a 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -658,7 +658,8 @@ static int l2tp_ip6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
return err < 0 ? err : len;
do_confirm:
- dst_confirm(dst);
+ if (msg->msg_flags & MSG_PROBE)
+ dst_confirm_neigh(dst, &fl6.daddr);
if (!(msg->msg_flags & MSG_PROBE) || len)
goto back_from_confirm;
err = 0;
--
1.9.3
^ permalink raw reply related
* [PATCHv2 RFC net-next 3/7] sctp: add dst_pending_confirm flag
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
In-Reply-To: <1485613578-19973-1-git-send-email-ja@ssi.bg>
Add new transport flag to allow sockets to confirm neighbour.
When same struct dst_entry can be used for many different
neighbours we can not use it for pending confirmations.
The flag is propagated from transport to every packet.
It is reset when cached dst is reset.
Reported-by: YueHaibing <yuehaibing@huawei.com>
Fixes: 5110effee8fd ("net: Do delayed neigh confirmation.")
Fixes: f2bb4bedf35d ("ipv4: Cache output routes in fib_info nexthops.")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/net/sctp/sctp.h | 6 ++----
include/net/sctp/structs.h | 4 ++++
net/sctp/associola.c | 3 +--
net/sctp/output.c | 10 +++++++++-
net/sctp/outqueue.c | 2 +-
net/sctp/sm_make_chunk.c | 6 ++----
net/sctp/sm_sideeffect.c | 2 +-
net/sctp/socket.c | 4 ++--
net/sctp/transport.c | 17 ++++++++++++++++-
9 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 3cfd365b..480b65a 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -593,10 +593,8 @@ static inline void sctp_v4_map_v6(union sctp_addr *addr)
*/
static inline struct dst_entry *sctp_transport_dst_check(struct sctp_transport *t)
{
- if (t->dst && !dst_check(t->dst, t->dst_cookie)) {
- dst_release(t->dst);
- t->dst = NULL;
- }
+ if (t->dst && !dst_check(t->dst, t->dst_cookie))
+ sctp_transport_dst_release(t);
return t->dst;
}
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 231fa9ac..6a68504 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -804,6 +804,8 @@ struct sctp_transport {
__u32 burst_limited; /* Holds old cwnd when max.burst is applied */
+ __u32 dst_pending_confirm; /* need to confirm neighbour */
+
/* Destination */
struct dst_entry *dst;
/* Source address. */
@@ -950,6 +952,8 @@ void sctp_transport_route(struct sctp_transport *, union sctp_addr *,
void sctp_transport_reset(struct sctp_transport *);
void sctp_transport_update_pmtu(struct sock *, struct sctp_transport *, u32);
void sctp_transport_immediate_rtx(struct sctp_transport *);
+void sctp_transport_dst_release(struct sctp_transport *t);
+void sctp_transport_dst_confirm(struct sctp_transport *t);
/* This is the structure we use to queue packets as they come into
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index e50dc6d..2a6835b 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -832,8 +832,7 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
if (transport->state != SCTP_UNCONFIRMED)
transport->state = SCTP_INACTIVE;
else {
- dst_release(transport->dst);
- transport->dst = NULL;
+ sctp_transport_dst_release(transport);
ulp_notify = false;
}
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 07ab506..814eac0 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -546,6 +546,7 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
struct sctp_association *asoc = tp->asoc;
struct sctp_chunk *chunk, *tmp;
int pkt_count, gso = 0;
+ int confirm;
struct dst_entry *dst;
struct sk_buff *head;
struct sctphdr *sh;
@@ -624,7 +625,14 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
asoc->peer.last_sent_to = tp;
}
head->ignore_df = packet->ipfragok;
- tp->af_specific->sctp_xmit(head, tp);
+ confirm = tp->dst_pending_confirm;
+ if (confirm)
+ skb_set_dst_pending_confirm(head, 1);
+ /* neighbour should be confirmed on successful transmission or
+ * positive error
+ */
+ if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
+ tp->dst_pending_confirm = 0;
out:
list_for_each_entry_safe(chunk, tmp, &packet->chunk_list, list) {
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 65abe22..db352e5 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -1654,7 +1654,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
if (forward_progress) {
if (transport->dst)
- dst_confirm(transport->dst);
+ sctp_transport_dst_confirm(transport);
}
}
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index ad3445b..c7d3249 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -3333,8 +3333,7 @@ static void sctp_asconf_param_success(struct sctp_association *asoc,
local_bh_enable();
list_for_each_entry(transport, &asoc->peer.transport_addr_list,
transports) {
- dst_release(transport->dst);
- transport->dst = NULL;
+ sctp_transport_dst_release(transport);
}
break;
case SCTP_PARAM_DEL_IP:
@@ -3348,8 +3347,7 @@ static void sctp_asconf_param_success(struct sctp_association *asoc,
local_bh_enable();
list_for_each_entry(transport, &asoc->peer.transport_addr_list,
transports) {
- dst_release(transport->dst);
- transport->dst = NULL;
+ sctp_transport_dst_release(transport);
}
break;
default:
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index a455271..51abcc9 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -755,7 +755,7 @@ static void sctp_cmd_transport_on(sctp_cmd_seq_t *cmds,
* forward progress.
*/
if (t->dst)
- dst_confirm(t->dst);
+ sctp_transport_dst_confirm(t);
/* The receiver of the HEARTBEAT ACK should also perform an
* RTT measurement for that destination transport address
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d699d2c..9e98c87 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -588,7 +588,7 @@ static int sctp_send_asconf_add_ip(struct sock *sk,
list_for_each_entry(trans,
&asoc->peer.transport_addr_list, transports) {
/* Clear the source and route cache */
- dst_release(trans->dst);
+ sctp_transport_dst_release(trans);
trans->cwnd = min(4*asoc->pathmtu, max_t(__u32,
2*asoc->pathmtu, 4380));
trans->ssthresh = asoc->peer.i.a_rwnd;
@@ -839,7 +839,7 @@ static int sctp_send_asconf_del_ip(struct sock *sk,
*/
list_for_each_entry(transport, &asoc->peer.transport_addr_list,
transports) {
- dst_release(transport->dst);
+ sctp_transport_dst_release(transport);
sctp_transport_route(transport, NULL,
sctp_sk(asoc->base.sk));
}
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index baa1ac0..df274ff 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -240,7 +240,7 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
{
/* If we don't have a fresh route, look one up */
if (!transport->dst || transport->dst->obsolete) {
- dst_release(transport->dst);
+ sctp_transport_dst_release(transport);
transport->af_specific->get_dst(transport, &transport->saddr,
&transport->fl, sk);
}
@@ -672,3 +672,18 @@ void sctp_transport_immediate_rtx(struct sctp_transport *t)
sctp_transport_hold(t);
}
}
+
+/* Drop dst */
+void sctp_transport_dst_release(struct sctp_transport *t)
+{
+ dst_release(t->dst);
+ t->dst = NULL;
+ t->dst_pending_confirm = 0;
+}
+
+/* Schedule neighbour confirm */
+void sctp_transport_dst_confirm(struct sctp_transport *t)
+{
+ t->dst_pending_confirm = 1;
+}
+
--
1.9.3
^ permalink raw reply related
* [PATCHv2 RFC net-next 5/7] net: add confirm_neigh method to dst_ops
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
In-Reply-To: <1485613578-19973-1-git-send-email-ja@ssi.bg>
Add confirm_neigh method to dst_ops and use it from IPv4 and IPv6
to lookup and confirm the neighbour. Its usage via the new helper
dst_confirm_neigh() should be restricted to MSG_PROBE users for
performance reasons.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/net/arp.h | 16 ++++++++++++++++
include/net/dst.h | 7 +++++++
include/net/dst_ops.h | 2 ++
include/net/ndisc.h | 17 +++++++++++++++++
net/ipv4/route.c | 19 +++++++++++++++++++
net/ipv6/route.c | 16 ++++++++++++++++
net/xfrm/xfrm_policy.c | 16 ++++++++++++++++
7 files changed, 93 insertions(+)
diff --git a/include/net/arp.h b/include/net/arp.h
index 5e0f891..65619a2 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h
@@ -35,6 +35,22 @@ static inline struct neighbour *__ipv4_neigh_lookup(struct net_device *dev, u32
return n;
}
+static inline void __ipv4_confirm_neigh(struct net_device *dev, u32 key)
+{
+ struct neighbour *n;
+
+ rcu_read_lock_bh();
+ n = __ipv4_neigh_lookup_noref(dev, key);
+ if (n) {
+ unsigned long now = jiffies;
+
+ /* avoid dirtying neighbour */
+ if (n->confirmed != now)
+ n->confirmed = now;
+ }
+ rcu_read_unlock_bh();
+}
+
void arp_init(void);
int arp_ioctl(struct net *net, unsigned int cmd, void __user *arg);
void arp_send(int type, int ptype, __be32 dest_ip,
diff --git a/include/net/dst.h b/include/net/dst.h
index 6835d22..3a3b34b 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -477,6 +477,13 @@ static inline struct neighbour *dst_neigh_lookup_skb(const struct dst_entry *dst
return IS_ERR(n) ? NULL : n;
}
+static inline void dst_confirm_neigh(const struct dst_entry *dst,
+ const void *daddr)
+{
+ if (dst->ops->confirm_neigh)
+ dst->ops->confirm_neigh(dst, daddr);
+}
+
static inline void dst_link_failure(struct sk_buff *skb)
{
struct dst_entry *dst = skb_dst(skb);
diff --git a/include/net/dst_ops.h b/include/net/dst_ops.h
index 8a2b66d..13f6d59 100644
--- a/include/net/dst_ops.h
+++ b/include/net/dst_ops.h
@@ -33,6 +33,8 @@ struct dst_ops {
struct neighbour * (*neigh_lookup)(const struct dst_entry *dst,
struct sk_buff *skb,
const void *daddr);
+ void (*confirm_neigh)(const struct dst_entry *dst,
+ const void *daddr);
struct kmem_cache *kmem_cachep;
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index d562a2f..8a02146 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -391,6 +391,23 @@ static inline struct neighbour *__ipv6_neigh_lookup(struct net_device *dev, cons
return n;
}
+static inline void __ipv6_confirm_neigh(struct net_device *dev,
+ const void *pkey)
+{
+ struct neighbour *n;
+
+ rcu_read_lock_bh();
+ n = __ipv6_neigh_lookup_noref(dev, pkey);
+ if (n) {
+ unsigned long now = jiffies;
+
+ /* avoid dirtying neighbour */
+ if (n->confirmed != now)
+ n->confirmed = now;
+ }
+ rcu_read_unlock_bh();
+}
+
int ndisc_init(void);
int ndisc_late_init(void);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 4b7c231..cb494a5 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -154,6 +154,7 @@ static u32 *ipv4_cow_metrics(struct dst_entry *dst, unsigned long old)
static struct neighbour *ipv4_neigh_lookup(const struct dst_entry *dst,
struct sk_buff *skb,
const void *daddr);
+static void ipv4_confirm_neigh(const struct dst_entry *dst, const void *daddr);
static struct dst_ops ipv4_dst_ops = {
.family = AF_INET,
@@ -168,6 +169,7 @@ static struct neighbour *ipv4_neigh_lookup(const struct dst_entry *dst,
.redirect = ip_do_redirect,
.local_out = __ip_local_out,
.neigh_lookup = ipv4_neigh_lookup,
+ .confirm_neigh = ipv4_confirm_neigh,
};
#define ECN_OR_COST(class) TC_PRIO_##class
@@ -461,6 +463,23 @@ static struct neighbour *ipv4_neigh_lookup(const struct dst_entry *dst,
return neigh_create(&arp_tbl, pkey, dev);
}
+static void ipv4_confirm_neigh(const struct dst_entry *dst, const void *daddr)
+{
+ struct net_device *dev = dst->dev;
+ const __be32 *pkey = daddr;
+ const struct rtable *rt;
+
+ rt = (const struct rtable *)dst;
+ if (rt->rt_gateway)
+ pkey = (const __be32 *)&rt->rt_gateway;
+ else if (!daddr ||
+ (rt->rt_flags &
+ (RTCF_MULTICAST | RTCF_BROADCAST | RTCF_LOCAL)))
+ return;
+
+ __ipv4_confirm_neigh(dev, *(__force u32 *)pkey);
+}
+
#define IP_IDENTS_SZ 2048u
static atomic_t *ip_idents __read_mostly;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4b1f0f9..c876940 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -217,6 +217,21 @@ static struct neighbour *ip6_neigh_lookup(const struct dst_entry *dst,
return neigh_create(&nd_tbl, daddr, dst->dev);
}
+static void ip6_confirm_neigh(const struct dst_entry *dst, const void *daddr)
+{
+ struct net_device *dev = dst->dev;
+ struct rt6_info *rt = (struct rt6_info *)dst;
+
+ daddr = choose_neigh_daddr(rt, NULL, daddr);
+ if (!daddr)
+ return;
+ if (dev->flags & (IFF_NOARP | IFF_LOOPBACK))
+ return;
+ if (ipv6_addr_is_multicast((const struct in6_addr *)daddr))
+ return;
+ __ipv6_confirm_neigh(dev, daddr);
+}
+
static struct dst_ops ip6_dst_ops_template = {
.family = AF_INET6,
.gc = ip6_dst_gc,
@@ -233,6 +248,7 @@ static struct neighbour *ip6_neigh_lookup(const struct dst_entry *dst,
.redirect = rt6_do_redirect,
.local_out = __ip6_local_out,
.neigh_lookup = ip6_neigh_lookup,
+ .confirm_neigh = ip6_confirm_neigh,
};
static unsigned int ip6_blackhole_mtu(const struct dst_entry *dst)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 177e208..c010ee0 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2856,6 +2856,20 @@ static struct neighbour *xfrm_neigh_lookup(const struct dst_entry *dst,
return dst->path->ops->neigh_lookup(dst, skb, daddr);
}
+static void xfrm_confirm_neigh(const struct dst_entry *dst, const void *daddr)
+{
+ const struct dst_entry *path = dst->path;
+
+ if (path == dst) {
+ dst->ops->confirm_neigh(dst, daddr);
+ } else {
+ /* daddr can be from different family and we need the
+ * tunnel address. How to get it?
+ */
+ path->ops->confirm_neigh(path, NULL);
+ }
+}
+
int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
{
int err = 0;
@@ -2882,6 +2896,8 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
dst_ops->link_failure = xfrm_link_failure;
if (likely(dst_ops->neigh_lookup == NULL))
dst_ops->neigh_lookup = xfrm_neigh_lookup;
+ if (likely(!dst_ops->confirm_neigh))
+ dst_ops->confirm_neigh = xfrm_confirm_neigh;
if (likely(afinfo->garbage_collect == NULL))
afinfo->garbage_collect = xfrm_garbage_collect_deferred;
rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family], afinfo);
--
1.9.3
^ permalink raw reply related
* [PATCHv2 RFC net-next 2/7] net: add dst_pending_confirm flag to skbuff
From: Julian Anastasov @ 2017-01-28 14:26 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, YueHaibing
In-Reply-To: <1485613578-19973-1-git-send-email-ja@ssi.bg>
Add new skbuff flag to allow protocols to confirm neighbour.
When same struct dst_entry can be used for many different
neighbours we can not use it for pending confirmations.
Add sock_confirm_neigh() helper to confirm the neighbour and
use it for IPv4, IPv6 and VRF before dst_neigh_output.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
drivers/net/vrf.c | 5 ++++-
include/linux/skbuff.h | 12 ++++++++++++
include/net/sock.h | 14 ++++++++++++++
net/ipv4/ip_output.c | 5 ++++-
net/ipv6/ip6_output.c | 1 +
5 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 264fc15..630eafd 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -378,6 +378,7 @@ static int vrf_finish_output6(struct net *net, struct sock *sk,
if (unlikely(!neigh))
neigh = __neigh_create(&nd_tbl, nexthop, dst->dev, false);
if (!IS_ERR(neigh)) {
+ sock_confirm_neigh(skb, neigh);
ret = dst_neigh_output(dst, neigh, skb);
rcu_read_unlock_bh();
return ret;
@@ -574,8 +575,10 @@ static int vrf_finish_output(struct net *net, struct sock *sk, struct sk_buff *s
neigh = __ipv4_neigh_lookup_noref(dev, nexthop);
if (unlikely(!neigh))
neigh = __neigh_create(&arp_tbl, &nexthop, dev, false);
- if (!IS_ERR(neigh))
+ if (!IS_ERR(neigh)) {
+ sock_confirm_neigh(skb, neigh);
ret = dst_neigh_output(dst, neigh, skb);
+ }
rcu_read_unlock_bh();
err:
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6f63b7e..3ac3c3b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -613,6 +613,7 @@ static inline bool skb_mstamp_after(const struct skb_mstamp *t1,
* @wifi_acked_valid: wifi_acked was set
* @wifi_acked: whether frame was acked on wifi or not
* @no_fcs: Request NIC to treat last 4 bytes as Ethernet FCS
+ * @dst_pending_confirm: need to confirm neighbour
* @napi_id: id of the NAPI struct this skb came from
* @secmark: security marking
* @mark: Generic packet mark
@@ -743,6 +744,7 @@ struct sk_buff {
__u8 csum_level:2;
__u8 csum_bad:1;
+ __u8 dst_pending_confirm:1;
#ifdef CONFIG_IPV6_NDISC_NODETYPE
__u8 ndisc_nodetype:2;
#endif
@@ -3694,6 +3696,16 @@ static inline bool skb_rx_queue_recorded(const struct sk_buff *skb)
return skb->queue_mapping != 0;
}
+static inline void skb_set_dst_pending_confirm(struct sk_buff *skb, u32 val)
+{
+ skb->dst_pending_confirm = val;
+}
+
+static inline bool skb_get_dst_pending_confirm(const struct sk_buff *skb)
+{
+ return skb->dst_pending_confirm != 0;
+}
+
static inline struct sec_path *skb_sec_path(struct sk_buff *skb)
{
#ifdef CONFIG_XFRM
diff --git a/include/net/sock.h b/include/net/sock.h
index e113786..1bc821e 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1821,6 +1821,20 @@ static inline void sk_dst_confirm(struct sock *sk)
sk->sk_dst_pending_confirm = 1;
}
+static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
+{
+ if (skb_get_dst_pending_confirm(skb)) {
+ struct sock *sk = skb->sk;
+ unsigned long now = jiffies;
+
+ /* avoid dirtying neighbour */
+ if (n->confirmed != now)
+ n->confirmed = now;
+ if (sk && sk->sk_dst_pending_confirm)
+ sk->sk_dst_pending_confirm = 0;
+ }
+}
+
bool sk_mc_loop(struct sock *sk);
static inline bool sk_can_gso(const struct sock *sk)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index fac275c4..27f1db7 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -222,7 +222,10 @@ static int ip_finish_output2(struct net *net, struct sock *sk, struct sk_buff *s
if (unlikely(!neigh))
neigh = __neigh_create(&arp_tbl, &nexthop, dev, false);
if (!IS_ERR(neigh)) {
- int res = dst_neigh_output(dst, neigh, skb);
+ int res;
+
+ sock_confirm_neigh(skb, neigh);
+ res = dst_neigh_output(dst, neigh, skb);
rcu_read_unlock_bh();
return res;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 38122d0..7d90cab 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -119,6 +119,7 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
if (unlikely(!neigh))
neigh = __neigh_create(&nd_tbl, nexthop, dst->dev, false);
if (!IS_ERR(neigh)) {
+ sock_confirm_neigh(skb, neigh);
ret = dst_neigh_output(dst, neigh, skb);
rcu_read_unlock_bh();
return ret;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Tejun Heo @ 2017-01-28 14:43 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andy Lutomirski, Alexei Starovoitov, David Ahern, Andy Lutomirski,
Network Development, David S. Miller, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <87wpdiejjg.fsf@xmission.com>
Hello, Eric.
On Thu, Jan 26, 2017 at 01:45:07PM +1300, Eric W. Biederman wrote:
> > Eric, does this sound okay to you? You're the authority on exposing
> > things like namespace ids to users.
>
> *Boggle* Things that run across all network namespaces break any kind
> of sense I have about thinking about them.
>
> Running across more than one network namespace by default seems very
> broken to me.
Can you explain why that is? Other namespaces don't behave this way.
For example, a PID namespace doesn't hide the processes at the system
level. It just gives additional nested names to the namespaced
processes and having objects visible at the system level is very
useful for monitoring and management. Are there inherent reasons why
network namespace should be very different from other namespaces in
this regard?
Thanks.
--
tejun
^ permalink raw reply
* Re: cls_matchall and port mirroring questions
From: Ido Schimmel @ 2017-01-28 14:19 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, jiri, Andrew Lunn, Vivien Didelot, yotamg
In-Reply-To: <f310a430-cd20-f0b0-801c-49376ca20acb@gmail.com>
On Thu, Jan 26, 2017 at 07:00:50PM -0800, Florian Fainelli wrote:
> Hi,
>
> As I am adding support for cls_matchall in the b53/bcm_sf2 drivers, I
> was looking into several, yet unrelated things:
>
> - mlxsw does not seem to specify whether the port used for capture
> remains usable, or blocks non-mirror traffic ingressing/egressing it, do
> we want a control knob for that? If not, what is a sensible default,
> block all non capture traffic?
Doesn't make sense to me to add such a default. It's up to the user.
> - do we have an updated man page for tc-matchall.8 that features how to
> use the statistical sampler too? b53 switches have a divider that allows
> us to select how many frames we want to receive (10 bit value).
https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=5c5670fae43027778e84b9d9ff3b9d91a10a8131
Yotam (Cced) already commented that he intends to send iproute patches.
> - b53 supports capture against a particular MAC SA or DA (or both), do
> we want to be able to control that somehow?
Can't you just use flower (for example) instead of matchall in that
case?
> What about Marvell switches, what can they do?
No idea :)
> - a fair amount of code dealing with the cls_matchall mirroring entry
> is not switch driver specific, in fact, the only things that are switch
> driver specific are:
> - list pointer where to store this entry (typically in the private
> network device context)
> - operation to check whether the device belongs to us (identical
> netdev_ops)
> - retrieval of the destination port number (to_port) which is also
> typically available in network device private context
>
> Do we want to move a fair amount of code into switchdev, treat
> cls_matchall entries as a specific switchdev object, and have drivers
> take over at the same level that mlxsw_sp_port_add_cls_matchall_mirror()
> currently starts?
I prefer the current way in which we re-use as many as possible core
APIs without adding switchdev-specific code. I don't have a concrete
argument against your proposal, though.
^ permalink raw reply
* Re: [PATCH net-next 4/4] net: dsa: bcm_sf2: Add support for ethtool::rxnfc
From: kbuild test robot @ 2017-01-28 16:27 UTC (permalink / raw)
To: Florian Fainelli
Cc: kbuild-all, netdev, davem, andrew, vivien.didelot, cphealy,
Florian Fainelli
In-Reply-To: <20170127210516.16704-6-f.fainelli@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 941 bytes --]
Hi Florian,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-dsa-bcm_sf2-CFP-support/20170128-052440
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> ERROR: "bcm_sf2_set_rxnfc" [drivers/net/dsa/bcm_sf2.ko] undefined!
>> ERROR: "bcm_sf2_get_rxnfc" [drivers/net/dsa/bcm_sf2.ko] undefined!
>> ERROR: "bcm_sf2_cfp_rst" [drivers/net/dsa/bcm_sf2.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39914 bytes --]
^ permalink raw reply
* Re: cls_matchall and port mirroring questions
From: Jiri Pirko @ 2017-01-28 15:55 UTC (permalink / raw)
To: Ido Schimmel
Cc: Florian Fainelli, netdev, jiri, Andrew Lunn, Vivien Didelot,
yotamg
In-Reply-To: <20170128141904.GB2620@splinter.mtl.com>
Sat, Jan 28, 2017 at 03:19:04PM CET, idosch@mellanox.com wrote:
>On Thu, Jan 26, 2017 at 07:00:50PM -0800, Florian Fainelli wrote:
>> Hi,
>>
>> As I am adding support for cls_matchall in the b53/bcm_sf2 drivers, I
>> was looking into several, yet unrelated things:
>>
>> - mlxsw does not seem to specify whether the port used for capture
>> remains usable, or blocks non-mirror traffic ingressing/egressing it, do
>> we want a control knob for that? If not, what is a sensible default,
>> block all non capture traffic?
>
>Doesn't make sense to me to add such a default. It's up to the user.
>
>> - do we have an updated man page for tc-matchall.8 that features how to
>> use the statistical sampler too? b53 switches have a divider that allows
>> us to select how many frames we want to receive (10 bit value).
>
>https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=5c5670fae43027778e84b9d9ff3b9d91a10a8131
>
>Yotam (Cced) already commented that he intends to send iproute patches.
>
>> - b53 supports capture against a particular MAC SA or DA (or both), do
>> we want to be able to control that somehow?
>
>Can't you just use flower (for example) instead of matchall in that
>case?
Definitelly. No extensions to matchall, as otherwise it could not be
called like that anylonger:)
>
>> What about Marvell switches, what can they do?
>
>No idea :)
>
>> - a fair amount of code dealing with the cls_matchall mirroring entry
>> is not switch driver specific, in fact, the only things that are switch
>> driver specific are:
>> - list pointer where to store this entry (typically in the private
>> network device context)
>> - operation to check whether the device belongs to us (identical
>> netdev_ops)
>> - retrieval of the destination port number (to_port) which is also
>> typically available in network device private context
>>
>> Do we want to move a fair amount of code into switchdev, treat
>> cls_matchall entries as a specific switchdev object, and have drivers
>> take over at the same level that mlxsw_sp_port_add_cls_matchall_mirror()
>> currently starts?
>
>I prefer the current way in which we re-use as many as possible core
>APIs without adding switchdev-specific code. I don't have a concrete
>argument against your proposal, though.
This (tc-offload) is completely unrelated to switchdev. So it would make
no sense.
^ permalink raw reply
* Re: cls_matchall and port mirroring questions
From: Florian Fainelli @ 2017-01-28 17:18 UTC (permalink / raw)
To: Jiri Pirko, Ido Schimmel
Cc: netdev, jiri, Andrew Lunn, Vivien Didelot, yotamg
In-Reply-To: <20170128155531.GA1770@nanopsycho>
Le 01/28/17 à 07:55, Jiri Pirko a écrit :
> Sat, Jan 28, 2017 at 03:19:04PM CET, idosch@mellanox.com wrote:
>> On Thu, Jan 26, 2017 at 07:00:50PM -0800, Florian Fainelli wrote:
>>> Hi,
>>>
>>> As I am adding support for cls_matchall in the b53/bcm_sf2 drivers, I
>>> was looking into several, yet unrelated things:
>>>
>>> - mlxsw does not seem to specify whether the port used for capture
>>> remains usable, or blocks non-mirror traffic ingressing/egressing it, do
>>> we want a control knob for that? If not, what is a sensible default,
>>> block all non capture traffic?
>>
>> Doesn't make sense to me to add such a default. It's up to the user.
>>
>>> - do we have an updated man page for tc-matchall.8 that features how to
>>> use the statistical sampler too? b53 switches have a divider that allows
>>> us to select how many frames we want to receive (10 bit value).
>>
>> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=5c5670fae43027778e84b9d9ff3b9d91a10a8131
>>
>> Yotam (Cced) already commented that he intends to send iproute patches.
>>
>>> - b53 supports capture against a particular MAC SA or DA (or both), do
>>> we want to be able to control that somehow?
>>
>> Can't you just use flower (for example) instead of matchall in that
>> case?
>
> Definitelly. No extensions to matchall, as otherwise it could not be
> called like that anylonger:)
OK, that make sense. I thought of matchall as action that would solely
deal with all port mirroring features, but using flower on top of an
ingress or egress qdisc + src/dst MAC would definitively and I guess
make senses too.
>
>
>>
>>> What about Marvell switches, what can they do?
>>
>> No idea :)
>>
>>> - a fair amount of code dealing with the cls_matchall mirroring entry
>>> is not switch driver specific, in fact, the only things that are switch
>>> driver specific are:
>>> - list pointer where to store this entry (typically in the private
>>> network device context)
>>> - operation to check whether the device belongs to us (identical
>>> netdev_ops)
>>> - retrieval of the destination port number (to_port) which is also
>>> typically available in network device private context
>>>
>>> Do we want to move a fair amount of code into switchdev, treat
>>> cls_matchall entries as a specific switchdev object, and have drivers
>>> take over at the same level that mlxsw_sp_port_add_cls_matchall_mirror()
>>> currently starts?
>>
>> I prefer the current way in which we re-use as many as possible core
>> APIs without adding switchdev-specific code. I don't have a concrete
>> argument against your proposal, though.
>
> This (tc-offload) is completely unrelated to switchdev. So it would make
> no sense.
That is true, there is a bit of code (allocation of tc entries, parsing
of actions list etc.) that could be made generic; and one could argue
that each tc action programming request could look like some kind of
special switchdev object. I am totally fine keeping things the way they
are though.
Thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 0/4] mlx5: Create build configuration options
From: Tom Herbert @ 2017-01-28 17:19 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Saeed Mahameed, David S. Miller, Linux Netdev List, Kernel Team
In-Reply-To: <CALzJLG8dh0TZPotrk-Xp6SZ=2UnjY2-C3TEQNxXDER5H8iwiQw@mail.gmail.com>
On Sat, Jan 28, 2017 at 3:38 AM, Saeed Mahameed
<saeedm@dev.mellanox.co.il> wrote:
> On Fri, Jan 27, 2017 at 8:13 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Fri, Jan 27, 2017 at 9:58 AM, Saeed Mahameed
>> <saeedm@dev.mellanox.co.il> wrote:
>>> On Fri, Jan 27, 2017 at 1:32 AM, Tom Herbert <tom@herbertland.com> wrote:
>>>> This patchset creates configuration options for sriov, vxlan, eswitch,
>>>> and tc features in the mlx5 driver. The purpose of this is to allow not
>>>> building these features. These features are optional advanced features
>>>> that are not required for a core Ethernet driver. A user can disable
>>>> these features which resuces the amount of code in the driver. Disabling
>>>> these features (and DCB) reduces the size of mlx5_core.o by about 16%.
>>>> This is also can reduce the complexity of backport and rebases since
>>>> user would no longer need to worry about dependencies with the rest of
>>>> the kernel that features which might not be of any interest to a user
>>>> may bring in.
>>>>
>>>> Tested: Build and ran the driver with all features enabled (the default)
>>>> and with none enabled (including DCB). Did not see any issues. I did
>>>> not explicity test operation of ayy of features in the list.
>>>>
>>>
>>> Basically I am not against this kind of change, infact i am with it,
>>> although I would have done some restructuring in the driver before i
>>> did such change ;), filling the code with ifdefs is not a neat thing.
>>>
>> If you wish, please take this as an RFC and feel free to structure the
>> code the right way. I think the intent is clear enough and looks like
>> davem isn't going to allow the directory restructuring so something
>> like this seems to be the best course of action now.
>>
>
> Right.
>
>>> I agree this will simplify backporting and provide some kind of
>>> feature separation inside the driver.
>>> But this will also increase the testing matrix we need to cover and
>>> increase the likelihood of kbuild breaks by an order of magnitude.
>>>
>> The testing matrix already exploded with the proliferation of
>> supported features. If anything this reduces the test matrix problem.
>> For instance, if we make a change to the core driver and functionality
>> properly isolated there is a much better chance that this won't affect
>> peripheral functionality and vice versa. It is just not feasible for
>> us to test every combination of NIC features for every change being
>> made.
>>
>
> Yes for isolated features, but for base functionality, we need to test
> it with all new device specific kconfig combinations on every patch!
Sorry, but that is the price you need to pay for a feature rich device.
On the subject of testing, I don't really see any indication in these
patches on how patches are being tested. Also, there are patches that
fix things without any mention of how to repro the problems. It is
critical that we know IPv6 is tested as much or more than IPv4 (just
last week with hit yet another IPv6-only issue in an another upstream
driver that should have been caught with a simple load test-- this
really is not acceptable any more!). Please add a description of how
patches were tested to commit logs.
Tom
> since a misplaced code inside or outside the correct ifdef
> can easily go unnoticed and break functionality.
>
>>> One more thing, do we really need a device specific flag per feature
>>> per vendor per device? can't we just use the same kconfig flag for
>>> all drivers and if there is a more generic system wide flag that
>>> covers the same feature
>>> can't we just use it, for instance instead of
>>> CONFIG_<DRIVER_NAME>_SRIOV why not use already existing CONFIG_PCI_IOV
>>> for all drivers ?
>>>
>> That sounds good to me. We already have CONFIG_RFS_ACCEL and others
>> that do that.
>>
>> Tom
>>
>>> Saeed.
>>>
>>>>
>>>>
>>>> Tom Herbert (4):
>>>> mlx5: Make building eswitch configurable
>>>> mlx5: Make building SR-IOV configurable
>>>> mlx5: Make building tc hardware offload configurable
>>>> mlx5: Make building vxlan hardware offload configurable
>>>>
>>>> drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 35 ++++++
>>>> drivers/net/ethernet/mellanox/mlx5/core/Makefile | 16 ++-
>>>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 129 ++++++++++++++++------
>>>> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 39 +++++--
>>>> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 4 +-
>>>> drivers/net/ethernet/mellanox/mlx5/core/lag.c | 2 +
>>>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 32 ++++--
>>>> drivers/net/ethernet/mellanox/mlx5/core/sriov.c | 6 +-
>>>> 8 files changed, 205 insertions(+), 58 deletions(-)
>>>>
>>>> --
>>>> 2.9.3
>>>>
^ permalink raw reply
* Re: [PATCH net-next v2 1/4] net: dsa: Add plumbing for port mirroring
From: Florian Fainelli @ 2017-01-28 17:20 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, andrew, vivien.didelot, cphealy
In-Reply-To: <20170128091449.GA2033@nanopsycho>
Le 01/28/17 à 01:14, Jiri Pirko a écrit :
> Sat, Jan 28, 2017 at 02:25:25AM CET, f.fainelli@gmail.com wrote:
>> Add necessary plumbing at the slave network device level to have switch
>> drivers implement ndo_setup_tc() and most particularly the cls_matchall
>> classifier. We add support for two switch operations:
>>
>> port_add_mirror and port_del_mirror() which configure, on a per-port
>> basis the mirror parameters requested from the cls_matchall classifier.
>>
>> Code is largely borrowed from the Mellanox Spectrum switch driver.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>
> [...]
>
>
>> +/*
>> + * Mirroring TC entry
>> + */
>> +struct dsa_mall_mirror_tc_entry {
>> + u8 to_local_port;
>> + bool ingress;
>> +};
>> +
>> +/*
>> + * TC matchall entry
>> + */
>
> Why are you using multiline comment format for single line comments?
There are precedents in that file, but I will remove it.
>
>
>> +struct dsa_mall_tc_entry {
>> + struct list_head list;
>> + unsigned long cookie;
>> + enum dsa_port_mall_action_type type;
>> + union {
>> + struct dsa_mall_mirror_tc_entry mirror;
>> + };
>> +};
>> +
>> +
>> struct dsa_port {
>> struct net_device *netdev;
>> struct device_node *dn;
>> @@ -370,6 +397,15 @@ struct dsa_switch_ops {
>> int (*port_mdb_dump)(struct dsa_switch *ds, int port,
>> struct switchdev_obj_port_mdb *mdb,
>> int (*cb)(struct switchdev_obj *obj));
>> +
>> + /*
>> + * TC integration
>> + */
>> + int (*port_mirror_add)(struct dsa_switch *ds, int port,
>> + struct dsa_mall_mirror_tc_entry *mirror,
>> + bool ingress);
>> + void (*port_mirror_del)(struct dsa_switch *ds, int port,
>> + struct dsa_mall_mirror_tc_entry *mirror);
>> };
>
> [...]
>
>
>> +static int dsa_slave_add_cls_matchall(struct net_device *dev,
>> + __be16 protocol,
>> + struct tc_cls_matchall_offload *cls,
>> + bool ingress)
>> +{
>> + struct dsa_slave_priv *p = netdev_priv(dev);
>> + struct dsa_mall_tc_entry *mall_tc_entry;
>> + struct dsa_switch *ds = p->parent;
>> + struct net *net = dev_net(dev);
>> + struct dsa_slave_priv *to_p;
>> + struct net_device *to_dev;
>> + const struct tc_action *a;
>> + int err = -EOPNOTSUPP;
>> + LIST_HEAD(actions);
>> + int ifindex;
>> +
>> + if (!ds->ops->port_mirror_add)
>> + return err;
>> +
>> + if (!tc_single_action(cls->exts)) {
>> + netdev_err(dev, "only singular actions are supported\n");
>
> Why you note the user in this case, but in case he tries to add
> non-supported action you don't note him?
Will remove that message.
>
>
>> + return err;
>> + }
>> +
>> + mall_tc_entry = kzalloc(sizeof(*mall_tc_entry), GFP_KERNEL);
>> + if (!mall_tc_entry)
>> + return -ENOMEM;
>> + mall_tc_entry->cookie = cls->cookie;
>
> Hmm, I believe that this allocation and initialization should go into
> the "is_mirred if". You can do the checks in advance. That would also
> make the error path simplier.
Yes good point, seems like you may want to do the same in mlxsw since
that part of the code was loosely based on that too.
Thanks Jiri!
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 1/4] mlx5: Make building eswitch configurable
From: Alexei Starovoitov @ 2017-01-28 17:52 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Tom Herbert, Or Gerlitz, Saeed Mahameed, David Miller,
Linux Netdev List, Kernel Team
In-Reply-To: <CALzJLG_abLhT7wcrtB97KzurJ2-=OZv11cELjoRW79cOeyjxgw@mail.gmail.com>
On 1/28/17 3:20 AM, Saeed Mahameed wrote:
> On Sat, Jan 28, 2017 at 1:23 AM, Alexei Starovoitov <ast@fb.com> wrote:
>> On 1/27/17 1:15 PM, Saeed Mahameed wrote:
>>>
>>> It is only mandatory for configurations that needs eswitch, where the
>>> driver has no way to know about them, for a good old bare metal box,
>>> eswitch is not needed.
>>>
>>> we can do some work to strip the l2 table logic - needed for PFs to
>>> work on multi-host - out of eswitch but again that would further
>>> complicate the driver code since eswitch will still need to update l2
>>> tables for VFs.
>>
>>
>> Saeed,
>> for multi-host setups every host in that multi-host doesn't
>> actually see the eswitch, no? Otherwise broken driver on one machine
>> can affect the other hosts in the same bundle? Please double check,
>
> each host (PF) has its own eswitch, and each eswitch lives in its own
> "steering-space"
> and it can't affect others.
>
>> since this is absolutely critical HW requirement.
>>
>
> The only shared HW resources between hosts (PFs) is the simple l2 table,
> and the only thing a host can ask from the l2 talbe (FW) is: "forward
> UC MAC to me", and it is the responsibility of the the driver eswitch
> to do so.
>
> the l2 table is created and managed by FW, SW eswitch can only request
> from FW, and the FW is trusted.
ok. clear. thanks for explaining.
Could you describe the sequence of function calls within mlx5
that does the assignment of uc mac for PF ?
since I'm missing where eswitch is involved.
I can see:
mlx5e_nic_enable | mlx5e_set_mac
queue_work(priv->wq, &priv->set_rx_mode_work);
mlx5e_set_rx_mode_work
mlx5e_apply_netdev_addr
mlx5e_add_l2_flow_rule
^ permalink raw reply
* [PATCH net] net: ethtool: add support for 2500BaseT and 5000BaseT link modes
From: Pavel Belous @ 2017-01-28 19:53 UTC (permalink / raw)
To: davem; +Cc: netdev, decot, linux-kernel
This patch introduce support for 2500BaseT and 5000BaseT link modes.
These modes are included in the new IEEE 802.3bz standard.
Signed-off-by: Pavel Belous <pavel.s.belous@gmail.com>
---
include/uapi/linux/ethtool.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index f0db778..3dc91a4 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1384,6 +1384,8 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_10000baseLR_Full_BIT = 44,
ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT = 45,
ETHTOOL_LINK_MODE_10000baseER_Full_BIT = 46,
+ ETHTOOL_LINK_MODE_2500baseT_Full_BIT = 47,
+ ETHTOOL_LINK_MODE_5000baseT_Full_BIT = 48,
/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
@@ -1393,7 +1395,7 @@ enum ethtool_link_mode_bit_indices {
*/
__ETHTOOL_LINK_MODE_LAST
- = ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+ = ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
};
#define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name) \
--
2.7.4
^ permalink raw reply related
* [PATCH V2 1/3] net: bgmac: allocate struct bgmac just once & don't copy it
From: Rafał Miłecki @ 2017-01-28 21:08 UTC (permalink / raw)
To: David S . Miller
Cc: Jon Mason, Florian Fainelli, Felix Fietkau, netdev,
Rafał Miłecki
In-Reply-To: <20170128210832.26174-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
So far were were allocating struct bgmac in 3 places: platform code,
bcma code and shared bgmac_enet_probe function. The reason for this was
bgmac_enet_probe:
1) Requiring early-filled struct bgmac
2) Calling alloc_etherdev on its own in order to use netdev_priv later
This solution got few drawbacks:
1) Was duplicating allocating code
2) Required copying early-filled struct
3) Resulted in platform/bcma code having access only to unused struct
Solve this situation by simply extracting some probe code into the new
bgmac_alloc function.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Add bgmac_alloc function instead of hacking alloc_etherdev and netdev_priv
Important: this patch depends on:
[PATCH net-next] net: add devm version of alloc_etherdev_mqs function
and is the first user of devm_alloc_etherdev.
---
drivers/net/ethernet/broadcom/bgmac-bcma.c | 4 +---
drivers/net/ethernet/broadcom/bgmac-platform.c | 2 +-
drivers/net/ethernet/broadcom/bgmac.c | 24 ++++++++++++++++--------
drivers/net/ethernet/broadcom/bgmac.h | 3 ++-
4 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index 4a4ffc0c4c65..9281abda4026 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -117,12 +117,11 @@ static int bgmac_probe(struct bcma_device *core)
u8 *mac;
int err;
- bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL);
+ bgmac = bgmac_alloc(&core->dev);
if (!bgmac)
return -ENOMEM;
bgmac->bcma.core = core;
- bgmac->dev = &core->dev;
bgmac->dma_dev = core->dma_dev;
bgmac->irq = core->irq;
@@ -307,7 +306,6 @@ static int bgmac_probe(struct bcma_device *core)
err1:
bcma_mdio_mii_unregister(bgmac->mii_bus);
err:
- kfree(bgmac);
bcma_set_drvdata(core, NULL);
return err;
diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index 6f736c19872f..805e6ed6c390 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -151,7 +151,7 @@ static int bgmac_probe(struct platform_device *pdev)
struct resource *regs;
const u8 *mac_addr;
- bgmac = devm_kzalloc(&pdev->dev, sizeof(*bgmac), GFP_KERNEL);
+ bgmac = bgmac_alloc(&pdev->dev);
if (!bgmac)
return -ENOMEM;
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 0e066dc6b8cc..632d4d7b5a5b 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1446,22 +1446,31 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac)
}
EXPORT_SYMBOL_GPL(bgmac_phy_connect_direct);
-int bgmac_enet_probe(struct bgmac *info)
+struct bgmac *bgmac_alloc(struct device *dev)
{
struct net_device *net_dev;
struct bgmac *bgmac;
- int err;
/* Allocation and references */
- net_dev = alloc_etherdev(sizeof(*bgmac));
+ net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
if (!net_dev)
- return -ENOMEM;
+ return NULL;
net_dev->netdev_ops = &bgmac_netdev_ops;
net_dev->ethtool_ops = &bgmac_ethtool_ops;
+
bgmac = netdev_priv(net_dev);
- memcpy(bgmac, info, sizeof(*bgmac));
+ bgmac->dev = dev;
bgmac->net_dev = net_dev;
+
+ return bgmac;
+}
+
+int bgmac_enet_probe(struct bgmac *bgmac)
+{
+ struct net_device *net_dev = bgmac->net_dev;
+ int err;
+
net_dev->irq = bgmac->irq;
SET_NETDEV_DEV(net_dev, bgmac->dev);
@@ -1488,7 +1497,7 @@ int bgmac_enet_probe(struct bgmac *info)
err = bgmac_dma_alloc(bgmac);
if (err) {
dev_err(bgmac->dev, "Unable to alloc memory for DMA\n");
- goto err_netdev_free;
+ goto err_out;
}
bgmac->int_mask = BGMAC_IS_ERRMASK | BGMAC_IS_RX | BGMAC_IS_TX_MASK;
@@ -1521,8 +1530,7 @@ int bgmac_enet_probe(struct bgmac *info)
phy_disconnect(net_dev->phydev);
err_dma_free:
bgmac_dma_free(bgmac);
-err_netdev_free:
- free_netdev(net_dev);
+err_out:
return err;
}
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 71f493f2451f..dfebaded3b52 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -517,7 +517,8 @@ struct bgmac {
int (*phy_connect)(struct bgmac *bgmac);
};
-int bgmac_enet_probe(struct bgmac *info);
+struct bgmac *bgmac_alloc(struct device *dev);
+int bgmac_enet_probe(struct bgmac *bgmac);
void bgmac_enet_remove(struct bgmac *bgmac);
void bgmac_adjust_link(struct net_device *net_dev);
int bgmac_phy_connect_direct(struct bgmac *bgmac);
--
2.11.0
^ permalink raw reply related
* [PATCH V2 0/3] net-next: use one struct bgmac & add PHY support
From: Rafał Miłecki @ 2017-01-28 21:08 UTC (permalink / raw)
To: David S . Miller
Cc: Jon Mason, Florian Fainelli, Felix Fietkau, netdev,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This patchset adds support for initializing PHY using PHY subsystem.
It's required e.g. for wireless access point devices that use bgmac
supported Ethernet device connected to some external PHY.
Implementing this required accessing phydev in bcma specific code which
wasn't possible with core code allocating struct bgmac on its own. This
is why I needed to modify alloc_etherdev usage first.
Rafał Miłecki (3):
net: bgmac: allocate struct bgmac just once & don't copy it
net: bgmac: drop struct bcma_mdio we don't need anymore
net: bgmac: use PHY subsystem for initializing PHY
drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 108 +++++++++++-------------
drivers/net/ethernet/broadcom/bgmac-bcma.c | 6 +-
drivers/net/ethernet/broadcom/bgmac-platform.c | 2 +-
drivers/net/ethernet/broadcom/bgmac.c | 24 ++++--
drivers/net/ethernet/broadcom/bgmac.h | 5 +-
5 files changed, 72 insertions(+), 73 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH V2 3/3] net: bgmac: use PHY subsystem for initializing PHY
From: Rafał Miłecki @ 2017-01-28 21:08 UTC (permalink / raw)
To: David S . Miller
Cc: Jon Mason, Florian Fainelli, Felix Fietkau, netdev,
Rafał Miłecki
In-Reply-To: <20170128210832.26174-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
This adds support for using bgmac with PHYs supported by standalone PHY
drivers. Having any PHY initialization in bgmac is hacky and shouldn't
be extended but rather removed if anyone has hardware to test it.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
index 9d9984999dce..6ce80cbcb48e 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -132,6 +132,10 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
struct bcma_chipinfo *ci = &bgmac->bcma.core->bus->chipinfo;
u8 i;
+ /* For some legacy hardware we do chipset-based PHY initialization here
+ * without even detecting PHY ID. It's hacky and should be cleaned as
+ * soon as someone can test it.
+ */
if (ci->id == BCMA_CHIP_ID_BCM5356) {
for (i = 0; i < 5; i++) {
bcma_mdio_phy_write(bgmac, i, 0x1f, 0x008b);
@@ -140,6 +144,7 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
bcma_mdio_phy_write(bgmac, i, 0x12, 0x2aaa);
bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
}
+ return;
}
if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg != 10) ||
(ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg != 10) ||
@@ -161,7 +166,12 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
bcma_mdio_phy_write(bgmac, i, 0x17, 0x9273);
bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
}
+ return;
}
+
+ /* For all other hw do initialization using PHY subsystem. */
+ if (bgmac->net_dev && bgmac->net_dev->phydev)
+ phy_init_hw(bgmac->net_dev->phydev);
}
/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyreset */
--
2.11.0
^ permalink raw reply related
* [PATCH V2 2/3] net: bgmac: drop struct bcma_mdio we don't need anymore
From: Rafał Miłecki @ 2017-01-28 21:08 UTC (permalink / raw)
To: David S . Miller
Cc: Jon Mason, Florian Fainelli, Felix Fietkau, netdev,
Rafał Miłecki
In-Reply-To: <20170128210832.26174-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
Adding struct bcma_mdio was a workaround for bcma code not having access
to the struct bgmac used in the core code. Now we don't duplicate this
struct we can just use it internally in bcma code.
This simplifies code & allows access to all bgmac driver details from
all places in bcma code.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 98 ++++++++++---------------
drivers/net/ethernet/broadcom/bgmac-bcma.c | 2 +-
drivers/net/ethernet/broadcom/bgmac.h | 2 +-
3 files changed, 42 insertions(+), 60 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
index 7c19c8e2bf91..9d9984999dce 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -12,11 +12,6 @@
#include <linux/brcmphy.h>
#include "bgmac.h"
-struct bcma_mdio {
- struct bcma_device *core;
- u8 phyaddr;
-};
-
static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
u32 value, int timeout)
{
@@ -37,7 +32,7 @@ static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
* PHY ops
**************************************************/
-static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
+static u16 bcma_mdio_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg)
{
struct bcma_device *core;
u16 phy_access_addr;
@@ -56,12 +51,12 @@ static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
BUILD_BUG_ON(BGMAC_PC_MCT_SHIFT != BCMA_GMAC_CMN_PC_MCT_SHIFT);
BUILD_BUG_ON(BGMAC_PC_MTE != BCMA_GMAC_CMN_PC_MTE);
- if (bcma_mdio->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
- core = bcma_mdio->core->bus->drv_gmac_cmn.core;
+ if (bgmac->bcma.core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+ core = bgmac->bcma.core->bus->drv_gmac_cmn.core;
phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
} else {
- core = bcma_mdio->core;
+ core = bgmac->bcma.core;
phy_access_addr = BGMAC_PHY_ACCESS;
phy_ctl_addr = BGMAC_PHY_CNTL;
}
@@ -87,7 +82,7 @@ static u16 bcma_mdio_phy_read(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg)
}
/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
-static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
+static int bcma_mdio_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg,
u16 value)
{
struct bcma_device *core;
@@ -95,12 +90,12 @@ static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
u16 phy_ctl_addr;
u32 tmp;
- if (bcma_mdio->core->id.id == BCMA_CORE_4706_MAC_GBIT) {
- core = bcma_mdio->core->bus->drv_gmac_cmn.core;
+ if (bgmac->bcma.core->id.id == BCMA_CORE_4706_MAC_GBIT) {
+ core = bgmac->bcma.core->bus->drv_gmac_cmn.core;
phy_access_addr = BCMA_GMAC_CMN_PHY_ACCESS;
phy_ctl_addr = BCMA_GMAC_CMN_PHY_CTL;
} else {
- core = bcma_mdio->core;
+ core = bgmac->bcma.core;
phy_access_addr = BGMAC_PHY_ACCESS;
phy_ctl_addr = BGMAC_PHY_CNTL;
}
@@ -110,8 +105,8 @@ static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
tmp |= phyaddr;
bcma_write32(core, phy_ctl_addr, tmp);
- bcma_write32(bcma_mdio->core, BGMAC_INT_STATUS, BGMAC_IS_MDIO);
- if (bcma_read32(bcma_mdio->core, BGMAC_INT_STATUS) & BGMAC_IS_MDIO)
+ bcma_write32(bgmac->bcma.core, BGMAC_INT_STATUS, BGMAC_IS_MDIO);
+ if (bcma_read32(bgmac->bcma.core, BGMAC_INT_STATUS) & BGMAC_IS_MDIO)
dev_warn(&core->dev, "Error setting MDIO int\n");
tmp = BGMAC_PA_START;
@@ -132,39 +127,39 @@ static int bcma_mdio_phy_write(struct bcma_mdio *bcma_mdio, u8 phyaddr, u8 reg,
}
/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyinit */
-static void bcma_mdio_phy_init(struct bcma_mdio *bcma_mdio)
+static void bcma_mdio_phy_init(struct bgmac *bgmac)
{
- struct bcma_chipinfo *ci = &bcma_mdio->core->bus->chipinfo;
+ struct bcma_chipinfo *ci = &bgmac->bcma.core->bus->chipinfo;
u8 i;
if (ci->id == BCMA_CHIP_ID_BCM5356) {
for (i = 0; i < 5; i++) {
- bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x008b);
- bcma_mdio_phy_write(bcma_mdio, i, 0x15, 0x0100);
- bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
- bcma_mdio_phy_write(bcma_mdio, i, 0x12, 0x2aaa);
- bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
+ bcma_mdio_phy_write(bgmac, i, 0x1f, 0x008b);
+ bcma_mdio_phy_write(bgmac, i, 0x15, 0x0100);
+ bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000f);
+ bcma_mdio_phy_write(bgmac, i, 0x12, 0x2aaa);
+ bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
}
}
if ((ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg != 10) ||
(ci->id == BCMA_CHIP_ID_BCM4749 && ci->pkg != 10) ||
(ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg != 9)) {
- struct bcma_drv_cc *cc = &bcma_mdio->core->bus->drv_cc;
+ struct bcma_drv_cc *cc = &bgmac->bcma.core->bus->drv_cc;
bcma_chipco_chipctl_maskset(cc, 2, ~0xc0000000, 0);
bcma_chipco_chipctl_maskset(cc, 4, ~0x80000000, 0);
for (i = 0; i < 5; i++) {
- bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
- bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5284);
- bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
- bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x0010);
- bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000f);
- bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x5296);
- bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x1073);
- bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9073);
- bcma_mdio_phy_write(bcma_mdio, i, 0x16, 0x52b6);
- bcma_mdio_phy_write(bcma_mdio, i, 0x17, 0x9273);
- bcma_mdio_phy_write(bcma_mdio, i, 0x1f, 0x000b);
+ bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000f);
+ bcma_mdio_phy_write(bgmac, i, 0x16, 0x5284);
+ bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
+ bcma_mdio_phy_write(bgmac, i, 0x17, 0x0010);
+ bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000f);
+ bcma_mdio_phy_write(bgmac, i, 0x16, 0x5296);
+ bcma_mdio_phy_write(bgmac, i, 0x17, 0x1073);
+ bcma_mdio_phy_write(bgmac, i, 0x17, 0x9073);
+ bcma_mdio_phy_write(bgmac, i, 0x16, 0x52b6);
+ bcma_mdio_phy_write(bgmac, i, 0x17, 0x9273);
+ bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
}
}
}
@@ -172,17 +167,17 @@ static void bcma_mdio_phy_init(struct bcma_mdio *bcma_mdio)
/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyreset */
static int bcma_mdio_phy_reset(struct mii_bus *bus)
{
- struct bcma_mdio *bcma_mdio = bus->priv;
- u8 phyaddr = bcma_mdio->phyaddr;
+ struct bgmac *bgmac = bus->priv;
+ u8 phyaddr = bgmac->phyaddr;
- if (bcma_mdio->phyaddr == BGMAC_PHY_NOREGS)
+ if (phyaddr == BGMAC_PHY_NOREGS)
return 0;
- bcma_mdio_phy_write(bcma_mdio, phyaddr, MII_BMCR, BMCR_RESET);
+ bcma_mdio_phy_write(bgmac, phyaddr, MII_BMCR, BMCR_RESET);
udelay(100);
- if (bcma_mdio_phy_read(bcma_mdio, phyaddr, MII_BMCR) & BMCR_RESET)
- dev_err(&bcma_mdio->core->dev, "PHY reset failed\n");
- bcma_mdio_phy_init(bcma_mdio);
+ if (bcma_mdio_phy_read(bgmac, phyaddr, MII_BMCR) & BMCR_RESET)
+ dev_err(bgmac->dev, "PHY reset failed\n");
+ bcma_mdio_phy_init(bgmac);
return 0;
}
@@ -202,16 +197,12 @@ static int bcma_mdio_mii_write(struct mii_bus *bus, int mii_id, int regnum,
return bcma_mdio_phy_write(bus->priv, mii_id, regnum, value);
}
-struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
+struct mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac)
{
- struct bcma_mdio *bcma_mdio;
+ struct bcma_device *core = bgmac->bcma.core;
struct mii_bus *mii_bus;
int err;
- bcma_mdio = kzalloc(sizeof(*bcma_mdio), GFP_KERNEL);
- if (!bcma_mdio)
- return ERR_PTR(-ENOMEM);
-
mii_bus = mdiobus_alloc();
if (!mii_bus) {
err = -ENOMEM;
@@ -221,15 +212,12 @@ struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
mii_bus->name = "bcma_mdio mii bus";
sprintf(mii_bus->id, "%s-%d-%d", "bcma_mdio", core->bus->num,
core->core_unit);
- mii_bus->priv = bcma_mdio;
+ mii_bus->priv = bgmac;
mii_bus->read = bcma_mdio_mii_read;
mii_bus->write = bcma_mdio_mii_write;
mii_bus->reset = bcma_mdio_phy_reset;
mii_bus->parent = &core->dev;
- mii_bus->phy_mask = ~(1 << phyaddr);
-
- bcma_mdio->core = core;
- bcma_mdio->phyaddr = phyaddr;
+ mii_bus->phy_mask = ~(1 << bgmac->phyaddr);
err = mdiobus_register(mii_bus);
if (err) {
@@ -242,23 +230,17 @@ struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr)
err_free_bus:
mdiobus_free(mii_bus);
err:
- kfree(bcma_mdio);
return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(bcma_mdio_mii_register);
void bcma_mdio_mii_unregister(struct mii_bus *mii_bus)
{
- struct bcma_mdio *bcma_mdio;
-
if (!mii_bus)
return;
- bcma_mdio = mii_bus->priv;
-
mdiobus_unregister(mii_bus);
mdiobus_free(mii_bus);
- kfree(bcma_mdio);
}
EXPORT_SYMBOL_GPL(bcma_mdio_mii_unregister);
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index 9281abda4026..5ef60d4f12b4 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -177,7 +177,7 @@ static int bgmac_probe(struct bcma_device *core)
if (!bgmac_is_bcm4707_family(core) &&
!(ci->id == BCMA_CHIP_ID_BCM53573 && core->core_unit == 1)) {
- mii_bus = bcma_mdio_mii_register(core, bgmac->phyaddr);
+ mii_bus = bcma_mdio_mii_register(bgmac);
if (IS_ERR(mii_bus)) {
err = PTR_ERR(mii_bus);
goto err;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index dfebaded3b52..ab2db76e4fb8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -523,7 +523,7 @@ void bgmac_enet_remove(struct bgmac *bgmac);
void bgmac_adjust_link(struct net_device *net_dev);
int bgmac_phy_connect_direct(struct bgmac *bgmac);
-struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr);
+struct mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac);
void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
static inline u32 bgmac_read(struct bgmac *bgmac, u16 offset)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH V2 1/3] net: bgmac: allocate struct bgmac just once & don't copy it
From: kbuild test robot @ 2017-01-28 23:40 UTC (permalink / raw)
To: Rafał Miłecki
Cc: kbuild-all, David S . Miller, Jon Mason, Florian Fainelli,
Felix Fietkau, netdev, Rafał Miłecki
In-Reply-To: <20170128210832.26174-2-zajec5@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1647 bytes --]
Hi Rafał,
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/net-next-use-one-struct-bgmac-add-PHY-support/20170129-062241
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_alloc':
>> drivers/net/ethernet/broadcom/bgmac.c:1455:12: error: implicit declaration of function 'devm_alloc_etherdev' [-Werror=implicit-function-declaration]
net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
^~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/broadcom/bgmac.c:1455:10: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
^
cc1: some warnings being treated as errors
vim +/devm_alloc_etherdev +1455 drivers/net/ethernet/broadcom/bgmac.c
1449 struct bgmac *bgmac_alloc(struct device *dev)
1450 {
1451 struct net_device *net_dev;
1452 struct bgmac *bgmac;
1453
1454 /* Allocation and references */
> 1455 net_dev = devm_alloc_etherdev(dev, sizeof(*bgmac));
1456 if (!net_dev)
1457 return NULL;
1458
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57922 bytes --]
^ permalink raw reply
* Re: [BUG/RFC] vhost: net: big endian viring access despite virtio 1
From: Michael S. Tsirkin @ 2017-01-29 0:35 UTC (permalink / raw)
To: Greg Kurz; +Cc: kvm, netdev, virtualization@lists.linux-foundation.org
In-Reply-To: <20170127143747.1313d9ee@bahia.lan>
On Fri, Jan 27, 2017 at 02:37:47PM +0100, Greg Kurz wrote:
> On Fri, 27 Jan 2017 13:24:13 +0100
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
>
> > On 01/26/2017 08:20 PM, Michael S. Tsirkin wrote:
> > > On Thu, Jan 26, 2017 at 06:39:14PM +0100, Halil Pasic wrote:
> > >>
> > >> Hi!
> > >>
> > >> Recently I have been investigating some strange migration problems on
> > >> s390x.
> > >>
> > >> It turned out under certain circumstances vhost_net corrupts avail.idx by
> > >> using wrong endianness.
> >
> > [..]
> >
> > >> -------------------------8<--------------
> > >> >From b26e2bbdc03832a0204ee2b42967a1b49a277dc8 Mon Sep 17 00:00:00 2001
> > >> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> > >> Date: Thu, 26 Jan 2017 00:06:15 +0100
> > >> Subject: [PATCH] vhost: remove useless/dangerous reset of is_le
> > >>
> > >> The reset of is_le does no good, but it contributes its fair share to a
> > >> bug in vhost_net, which occurs if we have some oldubufs when stopping and
> > >> setting a fd = -1 as a backend. Instead of doing something convoluted in
> > >> vhost_net, let's just get rid of the reset.
> > >>
> > >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> > >> Fixes: commit 2751c9882b94
> > >> ---
> > >> drivers/vhost/vhost.c | 4 +---
> > >> 1 file changed, 1 insertion(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > >> index d643260..08072a2 100644
> > >> --- a/drivers/vhost/vhost.c
> > >> +++ b/drivers/vhost/vhost.c
> > >> @@ -1714,10 +1714,8 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq)
> > >> int r;
> > >> bool is_le = vq->is_le;
> > >>
> > >> - if (!vq->private_data) {
> > >> - vhost_reset_is_le(vq);
> > >> + if (!vq->private_data)
> > >> return 0;
> > >> - }
> > >>
> > >> vhost_init_is_le(vq);
> > >
> > >
> > > I think you do need to reset it, just maybe within vhost_init_is_le.
> > >
> > > if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
> > > vq->is_le = true;
> > > else
> > > vhost_reset_is_le(vq);
> > >
> > >
> >
> > That is a very good point! I have overlooked that while the
> > CONFIG_VHOST_CROSS_ENDIAN_LEGACY variant
> >
> > static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > {
> > /* Note for legacy virtio: user_be is initialized at reset time
> > * according to the host endianness. If userspace does not set an
> > * explicit endianness, the default behavior is native endian, as
> > * expected by legacy virtio.
> > */
> > vq->is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq->user_be;
> > }
> >
> > is fine the other variant
> >
> > static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > {
> > if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
> > vq->is_le = true;
> > }
> > is a very strange initializer (makes assumptions about the state
> > to be initialized).
> >
> > I agree, setting native endianness there sounds very reasonable.
> >
> > I have a question regarding readability. IMHO the relationship
> > of reset_is_le and int_is_le is a bit confusing, and I'm afraid
> > it could become even more confusing with using reset in one of
> > the init_is_le's.
> >
> > How about we do the following?
> >
> > static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > {
> > if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
> > vq->is_le = true;
> > + else
> > + vq->is_le = virtio_legacy_is_little_endian();
> >
> > }
> >
> > static void vhost_reset_is_le(struct vhost_virtqueue *vq)
> > {
> > - vq->is_le = virtio_legacy_is_little_endian();
> > + vhost_init_is_le(vq);
> > }
> >
> > That way we would have correct endianness both after reset
> > and after init, I think :).
> >
>
> Yes, I think this is what we need.
>
> Cheers.
OK, pls test this patch.
> --
> Greg
>
> > Thank you very much!
> >
> > Halil
> >
> >
^ 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