Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Yafang Shao @ 2019-02-17 14:26 UTC (permalink / raw)
  To: davem
  Cc: daniel, edumazet, joe, xiyou.wangcong, netdev, shaoyafang,
	Yafang Shao
In-Reply-To: <1550413592-7877-1-git-send-email-laoar.shao@gmail.com>

SOCK_DEBUG() is a old facility for debugging.
If the user want to use it for debugging, the user must modify the
application first, that doesn't seem like a good way.
Now we have more powerful facilities, i.e. bpf or tracepoint, for this kind
of debugging purpose.
So we'd better disable it by default.
The reason why I don't remove it comepletely is that someone may still
would like to use it for debugging.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Suggested-by: Joe Perches <joe@perches.com>
---
 include/net/sock.h | 13 ++++++++-----
 net/core/sock.c    |  3 +++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 6679f3c..444e92f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -81,14 +81,17 @@
  */
 
 /* Define this to get the SOCK_DBG debugging facility. */
-#define SOCK_DEBUGGING
+/* #define SOCK_DEBUGGING */
 #ifdef SOCK_DEBUGGING
-#define SOCK_DEBUG(sk, msg...) do { if ((sk) && sock_flag((sk), SOCK_DBG)) \
-					printk(KERN_DEBUG msg); } while (0)
+#define SOCK_DEBUG(sk, fmt, ...)			\
+do {							\
+	if ((sk) && sock_flag((sk), SOCK_DBG))		\
+		printk(KERN_DEBUG fmt, ##__VA_ARGS__);	\
+} while (0)
 #else
 /* Validate arguments and do nothing */
-static inline __printf(2, 3)
-void SOCK_DEBUG(const struct sock *sk, const char *msg, ...)
+__printf(2, 3)
+static inline void SOCK_DEBUG(const struct sock *sk, const char *fmt, ...)
 {
 }
 #endif
diff --git a/net/core/sock.c b/net/core/sock.c
index 71ded4d..7c15835 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -753,6 +753,9 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 
 	switch (optname) {
 	case SO_DEBUG:
+		/* This option takes effect only when SOCK_DEBUGGING
+		 * is defined.
+		 */
 		if (val && !capable(CAP_NET_ADMIN))
 			ret = -EACCES;
 		else
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 net-next 1/2] tcp: clean up SOCK_DEBUG()
From: Yafang Shao @ 2019-02-17 14:26 UTC (permalink / raw)
  To: davem
  Cc: daniel, edumazet, joe, xiyou.wangcong, netdev, shaoyafang,
	Yafang Shao
In-Reply-To: <1550413592-7877-1-git-send-email-laoar.shao@gmail.com>

Per discussion with Daniel[1] and Eric[2], these SOCK_DEBUG() calles in
TCP are not needed now.
We'd better clean up it.

[1] https://patchwork.ozlabs.org/patch/1035573/
[2] https://patchwork.ozlabs.org/patch/1040533/

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/ipv4/tcp_input.c | 19 +------------------
 net/ipv6/tcp_ipv6.c  |  2 --
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 7a027dec..6d2750e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3595,7 +3595,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 	 * this segment (RFC793 Section 3.9).
 	 */
 	if (after(ack, tp->snd_nxt))
-		goto invalid_ack;
+		return -1;
 
 	if (after(ack, prior_snd_una)) {
 		flag |= FLAG_SND_UNA_ADVANCED;
@@ -3714,10 +3714,6 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 		tcp_process_tlp_ack(sk, ack, flag);
 	return 1;
 
-invalid_ack:
-	SOCK_DEBUG(sk, "Ack %u after %u:%u\n", ack, tp->snd_una, tp->snd_nxt);
-	return -1;
-
 old_ack:
 	/* If data was SACKed, tag it and see if we should send more data.
 	 * If data was DSACKed, see if we can undo a cwnd reduction.
@@ -3731,7 +3727,6 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 		tcp_xmit_recovery(sk, rexmit);
 	}
 
-	SOCK_DEBUG(sk, "Ack %u before %u:%u\n", ack, tp->snd_una, tp->snd_nxt);
 	return 0;
 }
 
@@ -4432,13 +4427,9 @@ static void tcp_ofo_queue(struct sock *sk)
 		rb_erase(&skb->rbnode, &tp->out_of_order_queue);
 
 		if (unlikely(!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt))) {
-			SOCK_DEBUG(sk, "ofo packet was already received\n");
 			tcp_drop(sk, skb);
 			continue;
 		}
-		SOCK_DEBUG(sk, "ofo requeuing : rcv_next %X seq %X - %X\n",
-			   tp->rcv_nxt, TCP_SKB_CB(skb)->seq,
-			   TCP_SKB_CB(skb)->end_seq);
 
 		tail = skb_peek_tail(&sk->sk_receive_queue);
 		eaten = tail && tcp_try_coalesce(sk, tail, skb, &fragstolen);
@@ -4502,8 +4493,6 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 	NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE);
 	seq = TCP_SKB_CB(skb)->seq;
 	end_seq = TCP_SKB_CB(skb)->end_seq;
-	SOCK_DEBUG(sk, "out of order segment: rcv_next %X seq %X - %X\n",
-		   tp->rcv_nxt, seq, end_seq);
 
 	p = &tp->out_of_order_queue.rb_node;
 	if (RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
@@ -4779,10 +4768,6 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
 
 	if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
 		/* Partial packet, seq < rcv_next < end_seq */
-		SOCK_DEBUG(sk, "partial packet: rcv_next %X seq %X - %X\n",
-			   tp->rcv_nxt, TCP_SKB_CB(skb)->seq,
-			   TCP_SKB_CB(skb)->end_seq);
-
 		tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, tp->rcv_nxt);
 
 		/* If window is closed, drop tail of packet. But after
@@ -5061,8 +5046,6 @@ static int tcp_prune_queue(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	SOCK_DEBUG(sk, "prune_queue: c=%x\n", tp->copied_seq);
-
 	NET_INC_STATS(sock_net(sk), LINUX_MIB_PRUNECALLED);
 
 	if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e51cda7..57ef69a1 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -220,8 +220,6 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 		u32 exthdrlen = icsk->icsk_ext_hdr_len;
 		struct sockaddr_in sin;
 
-		SOCK_DEBUG(sk, "connect: ipv4 mapped\n");
-
 		if (__ipv6_only_sock(sk))
 			return -ENETUNREACH;
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 net-next 0/2] clean up SOCK_DEBUG()
From: Yafang Shao @ 2019-02-17 14:26 UTC (permalink / raw)
  To: davem
  Cc: daniel, edumazet, joe, xiyou.wangcong, netdev, shaoyafang,
	Yafang Shao

Per discussion with Daniel[1] and Eric[2], these SOCK_DEBUG() calles in
TCP are not needed now.
We'd better clean up it.

Plus undefine SOCK_DEBUGGING by default.

[1] https://patchwork.ozlabs.org/patch/1035573/
[2] https://patchwork.ozlabs.org/patch/1040533/

Yafang Shao (2):
  tcp: clean up SOCK_DEBUG()
  net: sock: undefine SOCK_DEBUGGING

 include/net/sock.h   | 13 ++++++++-----
 net/core/sock.c      |  3 +++
 net/ipv4/tcp_input.c | 19 +------------------
 net/ipv6/tcp_ipv6.c  |  2 --
 4 files changed, 12 insertions(+), 25 deletions(-)

-- 
1.8.3.1


^ permalink raw reply

* [PATCH net-next 3/3] net: dsa: mv88e6xxx: defautl to multicast and unicast flooding
From: Russell King @ 2019-02-17 14:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <20190217142414.cjtmpi5y2l5rtdlb@shell.armlinux.org.uk>

Switches work by learning the MAC address for each attached station by
monitoring traffic from each station.  When a station sends a packet,
the switch records which port the MAC address is connected to.

With IPv4 networking, before communication commences with a neighbour,
an ARP packet is broadcasted to all stations asking for the MAC address
corresponding with the IPv4.  The desired station responds with an ARP
reply, and the ARP reply causes the switch to learn which port the
station is connected to.

With IPv6 networking, the situation is rather different.  Rather than
broadcasting ARP packets, a "neighbour solicitation" is multicasted
rather than broadcasted.  This multicast needs to reach the intended
station in order for the neighbour to be discovered.

Once a neighbour has been discovered, and entered into the sending
stations neighbour cache, communication can restart at a point later
without sending a new neighbour solicitation, even if the entry in
the neighbour cache is marked as stale.  This can be after the MAC
address has expired from the forwarding cache of the DSA switch -
when that occurs, there is a long pause in communication.

Our DSA implementation for mv88e6xxx switches has defaulted to having
multicast and unicast flooding disabled.  As per the above description,
this is fine for IPv4 networking, since the broadcasted ARP queries
will be sent to and received by all stations on the same network.
However, this breaks IPv6 very badly - blocking neighbour solicitations
and later causing connections to stall.

The defaults that the Linux bridge code expect from bridges are that
unknown unicast frames and unknown multicast frames are flooded to
all stations, which is at odds to the defaults adopted by our DSA
implementation for mv88e6xxx switches.

This commit enables by default flooding of both unknown unicast and
unknown multicast frames.  This means that mv88e6xxx DSA switches now
behave as per the bridge(8) man page, and IPv6 works flawlessly through
such a switch.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b75a865a293d..eb5e3d88374f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2144,13 +2144,14 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
 static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
 {
 	struct dsa_switch *ds = chip->ds;
-	bool flood;
 
-	/* Upstream ports flood frames with unknown unicast or multicast DA */
-	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
+	/* Linux bridges are expected to flood unknown multicast and
+	 * unicast frames to all ports - as per the defaults specified
+	 * in the iproute2 bridge(8) man page. Not doing this causes
+	 * stalls and failures with IPv6 over Marvell bridges. */
 	if (chip->info->ops->port_set_egress_floods)
 		return chip->info->ops->port_set_egress_floods(chip, port,
-							       flood, flood);
+							       true, true);
 
 	return 0;
 }
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Russell King @ 2019-02-17 14:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <20190217142414.cjtmpi5y2l5rtdlb@shell.armlinux.org.uk>

Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
the multicast and unicast flood properties to be controlled.  These
can be controlled on a per-port basis via commands such as:

	bridge link set dev lan1 flood on|off
	bridge link set dev lan1 mcast_flood on|off

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 32e7af5caa69..b75a865a293d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4692,6 +4692,37 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
 	return err;
 }
 
+static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
+				       unsigned long flags)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	bool unicast, multicast;
+	int ret = -EOPNOTSUPP;
+
+	unicast = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port) ||
+		flags & BR_FLOOD;
+	multicast = flags & BR_MCAST_FLOOD;
+
+	mutex_lock(&chip->reg_lock);
+	if (chip->info->ops->port_set_egress_floods)
+		ret = chip->info->ops->port_set_egress_floods(chip, port,
+							      unicast,
+							      multicast);
+	mutex_unlock(&chip->reg_lock);
+
+	return ret;
+}
+
+static unsigned long mv88e6xxx_bridge_flags_support(struct dsa_switch *ds)
+{
+	unsigned long support = 0;
+
+	if (chip->info->ops->port_set_egress_floods)
+		support |= BR_FLOOD | BR_MCAST_FLOOD;
+
+	return support;
+}
+
 static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
 	.probe			= mv88e6xxx_drv_probe,
@@ -4719,6 +4750,8 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.set_ageing_time	= mv88e6xxx_set_ageing_time,
 	.port_bridge_join	= mv88e6xxx_port_bridge_join,
 	.port_bridge_leave	= mv88e6xxx_port_bridge_leave,
+	.port_bridge_flags	= mv88e6xxx_port_bridge_flags,
+	.bridge_flags_support	= mv88e6xxx_bridge_flags_support,
 	.port_stp_state_set	= mv88e6xxx_port_stp_state_set,
 	.port_fast_age		= mv88e6xxx_port_fast_age,
 	.port_vlan_filtering	= mv88e6xxx_port_vlan_filtering,
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 1/3] net: dsa: add support for bridge flags
From: Russell King @ 2019-02-17 14:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <20190217142414.cjtmpi5y2l5rtdlb@shell.armlinux.org.uk>

The Linux bridge implementation allows various properties of the bridge
to be controlled, such as flooding unknown unicast and multicast frames.
This patch adds the necessary DSA infrastructure to allow the Linux
bridge support to control these properties for DSA switches.

We implement this by providing two new methods: one to get the switch-
wide support bitmask, and another to set the properties.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 include/net/dsa.h  |  3 +++
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 15 +++++++++++++++
 net/dsa/slave.c    |  6 ++++++
 4 files changed, 26 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7f2a668ef2cc..6cc1222aa2ca 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -400,6 +400,9 @@ struct dsa_switch_ops {
 	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
 				      u8 state);
 	void	(*port_fast_age)(struct dsa_switch *ds, int port);
+	int	(*port_bridge_flags)(struct dsa_switch *ds, int port,
+				     unsigned long);
+	unsigned long (*bridge_flags_support)(struct dsa_switch *ds);
 
 	/*
 	 * VLAN support
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 1f4972dab9f2..f4f99ec29f5d 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -160,6 +160,8 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
 		     struct switchdev_trans *trans);
 int dsa_port_mdb_del(const struct dsa_port *dp,
 		     const struct switchdev_obj_port_mdb *mdb);
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
+			  struct switchdev_trans *trans);
 int dsa_port_vlan_add(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan,
 		      struct switchdev_trans *trans);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2d7e01b23572..eb745041b1d9 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -177,6 +177,21 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
 	return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
 }
 
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
+			  struct switchdev_trans *trans)
+{
+	struct dsa_switch *ds = dp->ds;
+	int port = dp->index;
+
+	if (switchdev_trans_ph_prepare(trans))
+		return ds->ops->port_bridge_flags ? 0 : -EOPNOTSUPP;
+
+	if (ds->ops->port_bridge_flags)
+		ds->ops->port_bridge_flags(ds, port, flags);
+
+	return 0;
+}
+
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid)
 {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e5e7c04821b..af53cdd14a29 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -295,6 +295,9 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
 		ret = dsa_port_ageing_time(dp, attr->u.ageing_time, trans);
 		break;
+	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
+		ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, trans);
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
@@ -384,6 +387,9 @@ static int dsa_slave_port_attr_get(struct net_device *dev,
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
 		attr->u.brport_flags_support = 0;
+		if (ds->ops->bridge_flags_support)
+			attr->u.brport_flags_support |=
+				ds->ops->bridge_flags_support(ds);
 		break;
 	default:
 		return -EOPNOTSUPP;
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 0/3] net: dsa: mv88e6xxx: fix IPv6
From: Russell King - ARM Linux admin @ 2019-02-17 14:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev

We'have had some emails in private over this issue, this is my current
patch set rebased on top of net-next which provides working IPv6 (and
probably other protocols as well) over mv88e6xxx DSA switches.

The problem comes down to mv88e6xxx defaulting to not flood unknown
unicast and multicast datagrams, as they would be by dumb switches,
and as the Linux bridge code does by default.

These flood settings can be disabled via the Linux bridge code if it's
desired to make the switch behave more like a managed switch, eg, by
enabling the multicast querier.  However, the multicast querier
defaults to being disabled which effectively means that by default,
mv88e6xxx switches block all multicast traffic.  This is at odds with
the Linux bridge documentation, and the defaults that the Linux bridge
code adopts.

So, this patch set adds DSA support for Linux bridge flags, adds
mv88e6xxx support for the unicast and multicast flooding flags, and
lastly enables flooding of these frames by default to match the
Linux bridge defaults.

 drivers/net/dsa/mv88e6xxx/chip.c | 42 ++++++++++++++++++++++++++++++++++++----
 include/net/dsa.h                |  3 +++
 net/dsa/dsa_priv.h               |  2 ++
 net/dsa/port.c                   | 15 ++++++++++++++
 net/dsa/slave.c                  |  6 ++++++
 5 files changed, 64 insertions(+), 4 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH v2 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Yafang Shao @ 2019-02-17 14:07 UTC (permalink / raw)
  To: Joe Perches
  Cc: David Miller, Daniel Borkmann, Eric Dumazet, Cong Wang, netdev,
	shaoyafang
In-Reply-To: <3aa310cede544d76e53ec962676a6ea4e3bd6b23.camel@perches.com>

On Sun, Feb 17, 2019 at 7:58 PM Joe Perches <joe@perches.com> wrote:
>
> On Sun, 2019-02-17 at 00:28 +0800, Yafang Shao wrote:
> > SOCK_DEBUG() is a old facility for debugging.
> > If the user want to use it for debugging, the user must modify the
> > application first, that doesn't seem like a good way.
> > Now we have more powerful facilities, i.e. bpf or tracepoint, for this kind
> > of debugging purpose.
> > So we'd better disable it by default.
> > The reason why I don't remove it comepletely is that someone may still
> > would like to use it for debugging.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Suggested-by: Joe Perches <joe@perches.com>
> > ---
> >  include/net/sock.h | 13 ++++++++-----
> >  net/core/sock.c    |  3 +++
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 6679f3c..d41e8f4 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -81,14 +81,17 @@
> >   */
> >
> >  /* Define this to get the SOCK_DBG debugging facility. */
> > -#define SOCK_DEBUGGING
> > +/* #define SOCK_DEBUGGING */
> >  #ifdef SOCK_DEBUGGING
> > -#define SOCK_DEBUG(sk, msg...) do { if ((sk) && sock_flag((sk), SOCK_DBG)) \
> > -                                     printk(KERN_DEBUG msg); } while (0)
> > +#define SOCK_DEBUG(sk, fmt, ...)             \
> > +do {                                         \
> > +     if ((sk) && sock_flag((sk), SOCK_DBG))  \
> > +             pr_debug(fmt, ##__VA_ARGS__);   \
>
> trivia:
>
> I would not suggest pr_debug here as it also requires
> either DEBUG to be defined or CONFIG_DYNAMIC_DEBUG
> to be set.
>
> If you really set SOCK_DEBUGGING, then printk(KERN_DEBUG
> is probably right.
>

Sure. Will change it.

Thanks
Yafang

^ permalink raw reply

* linux-next: Fixes tags need some work in the net tree
From: Stephen Rothwell @ 2019-02-17 13:52 UTC (permalink / raw)
  To: David Miller, Networking
  Cc: Linux Next Mailing List, Linux Kernel Mailing List,
	Hauke Mehrtens, Florian Fainelli

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

Hi all,

In commit

  a40061ea2e39 ("net: systemport: Fix reception of BPDUs")

Fixes tag

  Fixes: 4e8aedfe78c7 ("net: systemport: Turn on offloads by default")

has these problem(s):

  - Target SHA1 does not exist
    Did you mean

  Fixes: b5061778f822 ("net: systemport: Turn on offloads by default")?

In commit

  3b89ea9c5902 ("net: Fix for_each_netdev_feature on Big endian")

Fixes tag

  Fixes: fd867d51f ("net/core: generic support for disabling netdev features down stack")

has these problem(s):

  - SHA1 should be at least 12 digits long
    Can be fixed by setting core.abbrev to 12 (or more) or (for git v2.11
    or later) just making sure it is not set (or set to "auto").

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH net-next 1/2] net/mlx5e: Add missing static function annotation
From: Leon Romanovsky @ 2019-02-17 13:21 UTC (permalink / raw)
  To: Saeed Mahameed, David S . Miller
  Cc: Leon Romanovsky, RDMA mailing list, Eyal Davidovich, linux-netdev
In-Reply-To: <20190217132128.9709-1-leon@kernel.org>

From: Leon Romanovsky <leonro@mellanox.com>


Compilation with W=1 produces following warning:

drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c:69:6:
warning: no previous prototype for _mlx5e_monitor_counter_start_ [-Wmissing-prototypes]
 void mlx5e_monitor_counter_start(struct mlx5e_priv *priv)
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Avoid it by declaring mlx5e_monitor_counter_start() as a static function.

Fixes: 5c7e8bbb0257 ("net/mlx5e: Use monitor counters for update stats")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c
index 2ce420851e77..7cd5b02e0f10 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c
@@ -66,7 +66,7 @@ static int mlx5e_monitor_event_handler(struct notifier_block *nb,
 	return NOTIFY_OK;
 }

-void mlx5e_monitor_counter_start(struct mlx5e_priv *priv)
+static void mlx5e_monitor_counter_start(struct mlx5e_priv *priv)
 {
 	MLX5_NB_INIT(&priv->monitor_counters_nb, mlx5e_monitor_event_handler,
 		     MONITOR_COUNTER);
--
2.19.1


^ permalink raw reply related

* [PATCH net-next 2/2] net/mlx5: Delete unused FPGA QPN variable
From: Leon Romanovsky @ 2019-02-17 13:21 UTC (permalink / raw)
  To: Saeed Mahameed, David S . Miller
  Cc: Leon Romanovsky, RDMA mailing list, Eyal Davidovich, linux-netdev
In-Reply-To: <20190217132128.9709-1-leon@kernel.org>

From: Leon Romanovsky <leonro@mellanox.com>

fpga_qpn was assigned but never used and compilation with W=1
produced the following warning:

drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c: In function _mlx5_fpga_event_:
drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c:320:6: warning:
variable _fpga_qpn_ set but not used [-Wunused-but-set-variable]
  u32 fpga_qpn;
      ^~~~~~~~

Fixes: 98db16bab59f ("net/mlx5: FPGA, Handle QP error event")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c
index 27c5f6c7d36a..d046d1ec2a86 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c
@@ -317,7 +317,6 @@ static int mlx5_fpga_event(struct mlx5_fpga_device *fdev,
 	const char *event_name;
 	bool teardown = false;
 	unsigned long flags;
-	u32 fpga_qpn;
 	u8 syndrome;

 	switch (event) {
@@ -328,7 +327,6 @@ static int mlx5_fpga_event(struct mlx5_fpga_device *fdev,
 	case MLX5_EVENT_TYPE_FPGA_QP_ERROR:
 		syndrome = MLX5_GET(fpga_qp_error_event, data, syndrome);
 		event_name = mlx5_fpga_qp_syndrome_to_string(syndrome);
-		fpga_qpn = MLX5_GET(fpga_qp_error_event, data, fpga_qpn);
 		break;
 	default:
 		return NOTIFY_DONE;
--
2.19.1


^ permalink raw reply related

* [PATCH net-next 0/2] Fix W=1 compilation warnings
From: Leon Romanovsky @ 2019-02-17 13:21 UTC (permalink / raw)
  To: Saeed Mahameed, David S . Miller
  Cc: Leon Romanovsky, RDMA mailing list, Eyal Davidovich, linux-netdev

From: Leon Romanovsky <leonro@mellanox.com>

Hi Saeed,

This short series fixes two small compilation warnings which are visible
while compiling with W=1.

Thanks

Leon Romanovsky (2):
  net/mlx5e: Add missing static function annotation
  net/mlx5: Delete unused FPGA QPN variable

 drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c        | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

--
2.19.1


^ permalink raw reply

* Re: [PATCH v2 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Joe Perches @ 2019-02-17 11:58 UTC (permalink / raw)
  To: Yafang Shao, davem; +Cc: daniel, edumazet, xiyou.wangcong, netdev, shaoyafang
In-Reply-To: <1550334537-380-3-git-send-email-laoar.shao@gmail.com>

On Sun, 2019-02-17 at 00:28 +0800, Yafang Shao wrote:
> SOCK_DEBUG() is a old facility for debugging.
> If the user want to use it for debugging, the user must modify the
> application first, that doesn't seem like a good way.
> Now we have more powerful facilities, i.e. bpf or tracepoint, for this kind
> of debugging purpose.
> So we'd better disable it by default.
> The reason why I don't remove it comepletely is that someone may still
> would like to use it for debugging.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Suggested-by: Joe Perches <joe@perches.com>
> ---
>  include/net/sock.h | 13 ++++++++-----
>  net/core/sock.c    |  3 +++
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 6679f3c..d41e8f4 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -81,14 +81,17 @@
>   */
>  
>  /* Define this to get the SOCK_DBG debugging facility. */
> -#define SOCK_DEBUGGING
> +/* #define SOCK_DEBUGGING */
>  #ifdef SOCK_DEBUGGING
> -#define SOCK_DEBUG(sk, msg...) do { if ((sk) && sock_flag((sk), SOCK_DBG)) \
> -					printk(KERN_DEBUG msg); } while (0)
> +#define SOCK_DEBUG(sk, fmt, ...)		\
> +do {						\
> +	if ((sk) && sock_flag((sk), SOCK_DBG))	\
> +		pr_debug(fmt, ##__VA_ARGS__);	\

trivia:

I would not suggest pr_debug here as it also requires
either DEBUG to be defined or CONFIG_DYNAMIC_DEBUG
to be set.

If you really set SOCK_DEBUGGING, then printk(KERN_DEBUG
is probably right.

                                                           


^ permalink raw reply

* [PATCH mlx5-next v1] net/mlx5: Factor out HCA capabilities functions
From: Leon Romanovsky @ 2019-02-17 11:11 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Saeed Mahameed, linux-netdev

From: Leon Romanovsky <leonro@mellanox.com>

Combine all HCA capabilities setters under one function
and compile out the ODP related function in case kernel
was compiled without ODP support.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
...
Changelog v0->v1:
 * Embedded config option check into ODP capability function flow
---
 .../net/ethernet/mellanox/mlx5/core/main.c    | 46 +++++++++++++------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 6d45518edbdc..025dfaf3466b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -466,7 +466,8 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
 	int set_sz;
 	int err;

-	if (!MLX5_CAP_GEN(dev, pg))
+	if (!IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING) ||
+	    !MLX5_CAP_GEN(dev, pg))
 		return 0;

 	err = mlx5_core_get_caps(dev, MLX5_CAP_ODP);
@@ -576,6 +577,33 @@ static int handle_hca_cap(struct mlx5_core_dev *dev)
 	return err;
 }

+static int set_hca_cap(struct mlx5_core_dev *dev)
+{
+	struct pci_dev *pdev = dev->pdev;
+	int err;
+
+	err = handle_hca_cap(dev);
+	if (err) {
+		dev_err(&pdev->dev, "handle_hca_cap failed\n");
+		goto out;
+	}
+
+	err = handle_hca_cap_atomic(dev);
+	if (err) {
+		dev_err(&pdev->dev, "handle_hca_cap_atomic failed\n");
+		goto out;
+	}
+
+	err = handle_hca_cap_odp(dev);
+	if (err) {
+		dev_err(&pdev->dev, "handle_hca_cap_odp failed\n");
+		goto out;
+	}
+
+out:
+	return err;
+}
+
 static int set_hca_ctrl(struct mlx5_core_dev *dev)
 {
 	struct mlx5_reg_host_endianness he_in;
@@ -963,21 +991,9 @@ static int mlx5_load_one(struct mlx5_core_dev *dev, struct mlx5_priv *priv,
 		goto reclaim_boot_pages;
 	}

-	err = handle_hca_cap(dev);
-	if (err) {
-		dev_err(&pdev->dev, "handle_hca_cap failed\n");
-		goto reclaim_boot_pages;
-	}
-
-	err = handle_hca_cap_atomic(dev);
-	if (err) {
-		dev_err(&pdev->dev, "handle_hca_cap_atomic failed\n");
-		goto reclaim_boot_pages;
-	}
-
-	err = handle_hca_cap_odp(dev);
+	err = set_hca_cap(dev);
 	if (err) {
-		dev_err(&pdev->dev, "handle_hca_cap_odp failed\n");
+		dev_err(&pdev->dev, "set_hca_cap failed\n");
 		goto reclaim_boot_pages;
 	}

--
2.19.1


^ permalink raw reply related

* Re: [Intel-wired-lan] [PATCH net-next] igc: Make function igc_write_rss_indir_tbl() static
From: Neftin, Sasha @ 2019-02-17 10:59 UTC (permalink / raw)
  To: Wei Yongjun, Jeff Kirsher; +Cc: netdev, kernel-janitors, intel-wired-lan
In-Reply-To: <6ef2f327-8dc6-4c86-f42d-4aa7508d19f5@intel.com>

On 2/17/2019 12:06, Neftin, Sasha wrote:
> On 2/16/2019 04:04, Wei Yongjun wrote:
>> Fixes the following sparse warning:
>>
>> drivers/net/ethernet/intel/igc/igc_ethtool.c:646:6: warning:
>>   symbol 'igc_write_rss_indir_tbl' was not declared. Should it be static?
>>
>> Fixes: 8c5ad0dae93c ("igc: Add ethtool support")
>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>> ---
>>   drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c 
>> b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> index eff37a6c0afa..544239422577 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> @@ -643,7 +643,7 @@ static int igc_set_coalesce(struct net_device 
>> *netdev,
>>       return 0;
>>   }
>> -void igc_write_rss_indir_tbl(struct igc_adapter *adapter)
>> +static void igc_write_rss_indir_tbl(struct igc_adapter *adapter)
>>   {
>>       struct igc_hw *hw = &adapter->hw;
>>       u32 reg = IGC_RETA(0);
>>
>>
>> NACK
> 'igc_write_rss_indir_tbl' method declared in igc.h file. This method 
> used in both igc_ethtool.c and igc_main.c and can't be a 'static'
> __
please, refer to commit 429abdf0cd3c... (Add multiple receive queues 
control supporting)


_____________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan


^ permalink raw reply

* Re: [PATCH RFC] net: bridge: don't flood known multicast traffic when snooping is enabled
From: Nikolay Aleksandrov @ 2019-02-17 10:58 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: roopa, wkok, anuradhak, bridge, linus.luessing, davem, stephen
In-Reply-To: <2bb7baaa-affb-451c-658d-bc5412a14c31@gmail.com>

On 17/02/2019 05:05, Florian Fainelli wrote:
> 
> 
> On 2/15/2019 5:04 AM, Nikolay Aleksandrov wrote:
>> The behaviour since b00589af3b04 ("bridge: disable snooping if there is
>> no querier") is wrong, we shouldn't be flooding multicast traffic when
>> there is an mdb entry and we know where it should be forwarded to when
>> multicast snooping is enabled. This patch changes the behaviour to not
>> flood known unicast traffic.
> 
> You mean multicast traffic in the last part of the sentence, right?
> 

Right. The change I wanted to discuss is when there is no querier and we have
a known mdst - forward only to its registered ports. The rest of the traffic
follows the current rules (i.e. no querier - flood unknown mcast). I'll send
an updated RFC version since this patch has issues as noted in the discussion
and we can continue from there.

>> I'll give two obviously broken cases:
>>  - most obvious: static mdb created by the user with snooping enabled
>>  - user-space daemon controlling the mdb table (e.g. MLAG)
>>
>> Every user would expect to have traffic forwarded only to the configured
>> mdb destination when snooping is enabled, instead now to get that one
>> needs to enable both snooping and querier. Enabling querier on all
>> switches could be problematic and is not a good solution, for example
>> as summarized by our multicast experts:
>> "every switch would send an IGMP query for any random multicast traffic it
>> received across the entire domain and it would send it forever as long as a
>> host exists wanting that stream even if it has no downstream/directly
>> connected receivers"
>>
>> Sending as an RFC to get the discussion going, but I'm strongly for
>> removing this behaviour and would like to send this patch officially.
>>
>> We could make this behaviour possible via a knob if necessary, but
>> it really should not be the default.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>>  net/bridge/br_device.c | 3 +--
>>  net/bridge/br_input.c  | 3 +--
>>  2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
>> index 013323b6dbe4..2aa8a6509924 100644
>> --- a/net/bridge/br_device.c
>> +++ b/net/bridge/br_device.c
>> @@ -96,8 +96,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>>  		}
>>  
>>  		mdst = br_mdb_get(br, skb, vid);
>> -		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
>> -		    br_multicast_querier_exists(br, eth_hdr(skb)))
>> +		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb))
>>  			br_multicast_flood(mdst, skb, false, true);
>>  		else
>>  			br_flood(br, skb, BR_PKT_MULTICAST, false, true);
>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>> index 5ea7e56119c1..aae78095cf67 100644
>> --- a/net/bridge/br_input.c
>> +++ b/net/bridge/br_input.c
>> @@ -136,8 +136,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>  	switch (pkt_type) {
>>  	case BR_PKT_MULTICAST:
>>  		mdst = br_mdb_get(br, skb, vid);
>> -		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
>> -		    br_multicast_querier_exists(br, eth_hdr(skb))) {
>> +		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
>>  			if ((mdst && mdst->host_joined) ||
>>  			    br_multicast_is_router(br)) {
>>  				local_rcv = true;
>>
> 


^ permalink raw reply

* Re: [PATCH net-next] igc: Make function igc_write_rss_indir_tbl() static
From: Neftin, Sasha @ 2019-02-17 10:06 UTC (permalink / raw)
  To: Wei Yongjun, Jeff Kirsher; +Cc: intel-wired-lan, netdev, kernel-janitors
In-Reply-To: <20190216020442.14206-1-weiyongjun1@huawei.com>

On 2/16/2019 04:04, Wei Yongjun wrote:
> Fixes the following sparse warning:
> 
> drivers/net/ethernet/intel/igc/igc_ethtool.c:646:6: warning:
>   symbol 'igc_write_rss_indir_tbl' was not declared. Should it be static?
> 
> Fixes: 8c5ad0dae93c ("igc: Add ethtool support")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> index eff37a6c0afa..544239422577 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> @@ -643,7 +643,7 @@ static int igc_set_coalesce(struct net_device *netdev,
>   	return 0;
>   }
>   
> -void igc_write_rss_indir_tbl(struct igc_adapter *adapter)
> +static void igc_write_rss_indir_tbl(struct igc_adapter *adapter)
>   {
>   	struct igc_hw *hw = &adapter->hw;
>   	u32 reg = IGC_RETA(0);
> 
> 
> NACK
'igc_write_rss_indir_tbl' method declared in igc.h file. This method 
used in both igc_ethtool.c and igc_main.c and can't be a 'static'

^ permalink raw reply

* [PATCH net-next v3 4/4] net: phy: marvell10g: check for newly set aneg
From: Heiner Kallweit @ 2019-02-17  9:32 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org
In-Reply-To: <46898fe1-8924-0da0-1bbd-b54c220ff8d1@gmail.com>

Even if the advertisement registers content didn't change, we may have
just switched to aneg, and therefore have to trigger an aneg restart.
This matches the behavior of genphy_config_aneg().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/marvell10g.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 895574083..b83eb19cf 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -296,6 +296,16 @@ static int mv3310_config_aneg(struct phy_device *phydev)
 	if (ret > 0)
 		changed = true;
 
+	if (!changed) {
+		/* Configure and restart aneg if it wasn't set before */
+		ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+		if (ret < 0)
+			return ret;
+
+		if (!(ret & MDIO_AN_CTRL1_ENABLE))
+			changed = 1;
+	}
+
 	if (changed)
 		ret = genphy_c45_restart_aneg(phydev);
 
-- 
2.20.1




^ permalink raw reply related

* [PATCH net-next v3 3/4] net: phy: marvell10g: use genphy_c45_an_config_aneg
From: Heiner Kallweit @ 2019-02-17  9:30 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org
In-Reply-To: <46898fe1-8924-0da0-1bbd-b54c220ff8d1@gmail.com>

From: Andrew Lunn <andrew@lunn.ch>
Use new function genphy_c45_config_aneg() in mv3310_config_aneg().

v2:
- add a comment regarding 1000BaseT vendor registers
v3:
- rebased

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
[hkallweit1@gmail.com: patch splitted]
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/marvell10g.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 4a6ae63ab..b7742799c 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -274,18 +274,15 @@ static int mv3310_config_aneg(struct phy_device *phydev)
 	if (phydev->autoneg == AUTONEG_DISABLE)
 		return genphy_c45_pma_setup_forced(phydev);
 
-	linkmode_and(phydev->advertising, phydev->advertising,
-		     phydev->supported);
-
-	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
-			     ADVERTISE_ALL | ADVERTISE_100BASE4 |
-			     ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
-			     linkmode_adv_to_mii_adv_t(phydev->advertising));
+	ret = genphy_c45_an_config_aneg(phydev);
 	if (ret < 0)
 		return ret;
 	if (ret > 0)
 		changed = true;
 
+	/* Clause 45 has no standardized support for 1000BaseT, therefore
+	 * use vendor registers for this mode.
+	 */
 	reg = linkmode_adv_to_mii_ctrl1000_t(phydev->advertising);
 	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MV_AN_CTRL1000,
 			     ADVERTISE_1000FULL | ADVERTISE_1000HALF, reg);
@@ -294,20 +291,6 @@ static int mv3310_config_aneg(struct phy_device *phydev)
 	if (ret > 0)
 		changed = true;
 
-	/* 10G control register */
-	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
-			      phydev->advertising))
-		reg = MDIO_AN_10GBT_CTRL_ADV10G;
-	else
-		reg = 0;
-
-	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
-				     MDIO_AN_10GBT_CTRL_ADV10G, reg);
-	if (ret < 0)
-		return ret;
-	if (ret > 0)
-		changed = true;
-
 	if (changed)
 		ret = genphy_c45_restart_aneg(phydev);
 
-- 
2.20.1



^ permalink raw reply related

* [PATCH net-next v3 2/4] net: phy: add genphy_c45_an_config_aneg
From: Heiner Kallweit @ 2019-02-17  9:29 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org
In-Reply-To: <46898fe1-8924-0da0-1bbd-b54c220ff8d1@gmail.com>

From: Andrew Lunn <andrew@lunn.ch>
C45 configuration of 10/100 and multi-giga bit auto negotiation
advertisement is standardized. Configuration of 1000Base-T however
appears to be vendor specific. Move the generic code out of the
Marvell driver into the common phy-c45.c file.

v2:
- change function name to genphy_c45_an_config_aneg

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
[hkallweit1@gmail.com: use new helper linkmode_adv_to_mii_10gbt_adv_t and split patch]
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy-c45.c | 44 +++++++++++++++++++++++++++++++++++++++
 include/linux/phy.h       |  1 +
 2 files changed, 45 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 98a04d4cd..16636d49b 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -78,6 +78,50 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced);
 
+/**
+ * genphy_c45_an_config_aneg - configure advertisement registers
+ * @phydev: target phy_device struct
+ *
+ * Configure advertisement registers based on modes set in phydev->advertising
+ *
+ * Returns negative errno code on failure, 0 if advertisement didn't change,
+ * or 1 if advertised modes changed.
+ */
+int genphy_c45_an_config_aneg(struct phy_device *phydev)
+{
+	int changed = 0, ret;
+	u32 adv;
+
+	linkmode_and(phydev->advertising, phydev->advertising,
+		     phydev->supported);
+
+	adv = linkmode_adv_to_mii_adv_t(phydev->advertising);
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
+			     ADVERTISE_ALL | ADVERTISE_100BASE4 |
+			     ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
+			     adv);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = 1;
+
+	adv = linkmode_adv_to_mii_10gbt_adv_t(phydev->advertising);
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
+			     MDIO_AN_10GBT_CTRL_ADV10G |
+			     MDIO_AN_10GBT_CTRL_ADV5G |
+			     MDIO_AN_10GBT_CTRL_ADV2_5G,
+			     adv);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = 1;
+
+	return changed;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_an_config_aneg);
+
 /**
  * genphy_c45_an_disable_aneg - disable auto-negotiation
  * @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index bf1070c2a..3db507e68 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1101,6 +1101,7 @@ int genphy_c45_read_link(struct phy_device *phydev);
 int genphy_c45_read_lpa(struct phy_device *phydev);
 int genphy_c45_read_pma(struct phy_device *phydev);
 int genphy_c45_pma_setup_forced(struct phy_device *phydev);
+int genphy_c45_an_config_aneg(struct phy_device *phydev);
 int genphy_c45_an_disable_aneg(struct phy_device *phydev);
 int genphy_c45_read_mdix(struct phy_device *phydev);
 int genphy_c45_pma_read_abilities(struct phy_device *phydev);
-- 
2.20.1




^ permalink raw reply related

* [PATCH net-next v3 1/4] net: phy: add helper linkmode_adv_to_mii_10gbt_adv_t
From: Heiner Kallweit @ 2019-02-17  9:28 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org
In-Reply-To: <46898fe1-8924-0da0-1bbd-b54c220ff8d1@gmail.com>

Add a helper linkmode_adv_to_mii_10gbt_adv_t(), similar to
linkmode_adv_to_mii_adv_t.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/linux/mdio.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 5b872c45f..5a65f32d8 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -280,6 +280,31 @@ static inline void mii_10gbt_stat_mod_linkmode_lpa_t(unsigned long *advertising,
 			 advertising, lpa & MDIO_AN_10GBT_STAT_LP10G);
 }
 
+/**
+ * linkmode_adv_to_mii_10gbt_adv_t
+ * @advertising: the linkmode advertisement settings
+ *
+ * A small helper function that translates linkmode advertisement
+ * settings to phy autonegotiation advertisements for the C45
+ * 10GBASE-T AN CONTROL (7.32) register.
+ */
+static inline u32 linkmode_adv_to_mii_10gbt_adv_t(unsigned long *advertising)
+{
+	u32 result = 0;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+			      advertising))
+		result |= MDIO_AN_10GBT_CTRL_ADV2_5G;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+			      advertising))
+		result |= MDIO_AN_10GBT_CTRL_ADV5G;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+			      advertising))
+		result |= MDIO_AN_10GBT_CTRL_ADV10G;
+
+	return result;
+}
+
 int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
 int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
 
-- 
2.20.1




^ permalink raw reply related

* [PATCH net-next v3 0/4] net: phy: add and use genphy_c45_an_config_an
From: Heiner Kallweit @ 2019-02-17  9:27 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org

This series adds genphy_c45_an_config_an() and uses it in the
marvell10g diver. In addition patch 4 aligns the aneg configuration
with what is done in genphy_config_aneg().

v2:
- in patch 2 changed function name to genphy_c45_an_config_aneg
- in patch 3 add a comment regarding 1000BaseT vendor registers

v3:
- rebase patch 3

Andrew Lunn (2):
  net: phy: add genphy_c45_an_config_an
  net: phy: marvell10g: use genphy_c45_an_config_aneg

Heiner Kallweit (2):
  net: phy: add helper linkmode_adv_to_mii_10gbt_adv_t
  net: phy: marvell10g: check for newly set aneg

 drivers/net/phy/marvell10g.c | 31 ++++++++++---------------
 drivers/net/phy/phy-c45.c    | 44 ++++++++++++++++++++++++++++++++++++
 include/linux/mdio.h         | 25 ++++++++++++++++++++
 include/linux/phy.h          |  1 +
 4 files changed, 82 insertions(+), 19 deletions(-)

-- 
2.20.1



^ permalink raw reply

* Re: [PATCH net-next v2 0/4] net: phy: add and use genphy_c45_an_config_an
From: Heiner Kallweit @ 2019-02-17  9:26 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org
In-Reply-To: <ad6530be-336a-1ede-e6d8-e2a016a8d43f@gmail.com>

On 17.02.2019 09:35, Heiner Kallweit wrote:
> This series adds genphy_c45_an_config_an() and uses it in the
> marvell10g diver. In addition patch 4 aligns the aneg configuration
> with what is done in genphy_config_aneg().
> 
> v2:
> - in patch 2 changed function name to genphy_c45_an_config_aneg
> - in patch 3 add a comment regarding 1000BaseT vendor registers
> 
> Andrew Lunn (2):
>   net: phy: add genphy_c45_an_config_an
>   net: phy: marvell10g: use genphy_c45_an_config_aneg
> 
> Heiner Kallweit (2):
>   net: phy: add helper linkmode_adv_to_mii_10gbt_adv_t
>   net: phy: marvell10g: check for newly set aneg
> 
>  drivers/net/phy/marvell10g.c | 31 ++++++++++---------------
>  drivers/net/phy/phy-c45.c    | 44 ++++++++++++++++++++++++++++++++++++
>  include/linux/mdio.h         | 25 ++++++++++++++++++++
>  include/linux/phy.h          |  1 +
>  4 files changed, 82 insertions(+), 19 deletions(-)
> 
Forgot to rebase patch 3 what's needed due to another pending patch.
Will send a v3.

^ permalink raw reply

* [PATCH net-next v2 4/4] net: phy: marvell10g: check for newly set aneg
From: Heiner Kallweit @ 2019-02-17  8:40 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org
In-Reply-To: <ad6530be-336a-1ede-e6d8-e2a016a8d43f@gmail.com>

Even if the advertisement registers content didn't change, we may have
just switched to aneg, and therefore have to trigger an aneg restart.
This matches the behavior of genphy_config_aneg().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/marvell10g.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 895574083..b83eb19cf 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -296,6 +296,16 @@ static int mv3310_config_aneg(struct phy_device *phydev)
 	if (ret > 0)
 		changed = true;
 
+	if (!changed) {
+		/* Configure and restart aneg if it wasn't set before */
+		ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+		if (ret < 0)
+			return ret;
+
+		if (!(ret & MDIO_AN_CTRL1_ENABLE))
+			changed = 1;
+	}
+
 	if (changed)
 		ret = genphy_c45_restart_aneg(phydev);
 
-- 
2.20.1



^ permalink raw reply related

* [PATCH net-next v2 3/4] net: phy: marvell10g: use genphy_c45_an_config_aneg
From: Heiner Kallweit @ 2019-02-17  8:39 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Russell King - ARM Linux
  Cc: netdev@vger.kernel.org
In-Reply-To: <ad6530be-336a-1ede-e6d8-e2a016a8d43f@gmail.com>

From: Andrew Lunn <andrew@lunn.ch>
Use new function genphy_c45_config_aneg() in mv3310_config_aneg().

v2:
- add a comment regarding 1000BaseT vendor registers

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
[hkallweit1@gmail.com: patch splitted]
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/marvell10g.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 496805c0d..895574083 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -279,18 +279,15 @@ static int mv3310_config_aneg(struct phy_device *phydev)
 		return genphy_c45_an_disable_aneg(phydev);
 	}
 
-	linkmode_and(phydev->advertising, phydev->advertising,
-		     phydev->supported);
-
-	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
-			     ADVERTISE_ALL | ADVERTISE_100BASE4 |
-			     ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
-			     linkmode_adv_to_mii_adv_t(phydev->advertising));
+	ret = genphy_c45_an_config_aneg(phydev);
 	if (ret < 0)
 		return ret;
 	if (ret > 0)
 		changed = true;
 
+	/* Clause 45 has no standardized support for 1000BaseT, therefore
+	 * use vendor registers for this mode.
+	 */
 	reg = linkmode_adv_to_mii_ctrl1000_t(phydev->advertising);
 	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MV_AN_CTRL1000,
 			     ADVERTISE_1000FULL | ADVERTISE_1000HALF, reg);
@@ -299,20 +296,6 @@ static int mv3310_config_aneg(struct phy_device *phydev)
 	if (ret > 0)
 		changed = true;
 
-	/* 10G control register */
-	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
-			      phydev->advertising))
-		reg = MDIO_AN_10GBT_CTRL_ADV10G;
-	else
-		reg = 0;
-
-	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
-				     MDIO_AN_10GBT_CTRL_ADV10G, reg);
-	if (ret < 0)
-		return ret;
-	if (ret > 0)
-		changed = true;
-
 	if (changed)
 		ret = genphy_c45_restart_aneg(phydev);
 
-- 
2.20.1



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox