Netdev List
 help / color / mirror / Atom feed
* [PATCH v4 net-next 2/4] net/sched: act_mirred: Refactor detection whether dev needs xmit at mac header
From: Shmulik Ladkani @ 2016-10-13  6:06 UTC (permalink / raw)
  To: David Miller
  Cc: Jamal Hadi Salim, Eric Dumazet, WANG Cong, Daniel Borkmann,
	netdev, Shmulik Ladkani
In-Reply-To: <1476338804-25440-1-git-send-email-shmulik.ladkani@gmail.com>

Move detection logic that tests whether device expects skb data to point
at mac_header upon xmit into a function.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
 net/sched/act_mirred.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 16e17a8..69dcce8 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -54,6 +54,20 @@ static void tcf_mirred_release(struct tc_action *a, int bind)
 static int mirred_net_id;
 static struct tc_action_ops act_mirred_ops;
 
+static bool dev_is_mac_header_xmit(const struct net_device *dev)
+{
+	switch (dev->type) {
+	case ARPHRD_TUNNEL:
+	case ARPHRD_TUNNEL6:
+	case ARPHRD_SIT:
+	case ARPHRD_IPGRE:
+	case ARPHRD_VOID:
+	case ARPHRD_NONE:
+		return false;
+	}
+	return true;
+}
+
 static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 			   struct nlattr *est, struct tc_action **a, int ovr,
 			   int bind)
@@ -96,19 +110,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 				tcf_hash_release(*a, bind);
 			return -ENODEV;
 		}
-		switch (dev->type) {
-		case ARPHRD_TUNNEL:
-		case ARPHRD_TUNNEL6:
-		case ARPHRD_SIT:
-		case ARPHRD_IPGRE:
-		case ARPHRD_VOID:
-		case ARPHRD_NONE:
-			mac_header_xmit = false;
-			break;
-		default:
-			mac_header_xmit = true;
-			break;
-		}
+		mac_header_xmit = dev_is_mac_header_xmit(dev);
 	} else {
 		dev = NULL;
 	}
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 net-next 0/4] act_mirred: Ingress actions support
From: Shmulik Ladkani @ 2016-10-13  6:06 UTC (permalink / raw)
  To: David Miller
  Cc: Jamal Hadi Salim, Eric Dumazet, WANG Cong, Daniel Borkmann,
	netdev, Shmulik Ladkani

This patch series implements action mirred 'ingress' actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.

This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.

v4:
  in 4/4, check ret code of netif_receive_skb, as suggested by Cong Wang
v3:
  in 4/4, addressed non coherency due to reading m->tcfm_eaction multiple
  times, as spotted by Eric Dumazet
v2:
  in 1/4, declare tcfm_mac_header_xmit as bool instead of int

Shmulik Ladkani (4):
  net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit and
    make it a bool
  net/sched: act_mirred: Refactor detection whether dev needs xmit at
    mac header
  net/sched: tc_mirred: Rename public predicates
    'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
  net/sched: act_mirred: Implement ingress actions

 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c  |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  4 +-
 .../net/ethernet/netronome/nfp/nfp_net_offload.c   |  2 +-
 include/net/tc_act/tc_mirred.h                     |  6 +-
 net/sched/act_mirred.c                             | 84 ++++++++++++++++------
 7 files changed, 73 insertions(+), 29 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH v4 net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Shmulik Ladkani @ 2016-10-13  6:06 UTC (permalink / raw)
  To: David Miller
  Cc: Jamal Hadi Salim, Eric Dumazet, WANG Cong, Daniel Borkmann,
	netdev, Shmulik Ladkani, Eric Dumazet
In-Reply-To: <1476338804-25440-1-git-send-email-shmulik.ladkani@gmail.com>

Up until now, 'action mirred' supported only egress actions (either
TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).

This patch implements the corresponding ingress actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.

This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
---
 v4: check ret code of netif_receive_skb, as suggested by Cong Wang

 v3: Addressed non coherency due to reading m->tcfm_eaction multiple times,
     as spotted by Eric Dumazet

 net/sched/act_mirred.c | 51 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 69dcce8..2d93be6 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -33,6 +33,25 @@
 static LIST_HEAD(mirred_list);
 static DEFINE_SPINLOCK(mirred_list_lock);
 
+static bool tcf_mirred_is_act_redirect(int action)
+{
+	return action == TCA_EGRESS_REDIR || action == TCA_INGRESS_REDIR;
+}
+
+static u32 tcf_mirred_act_direction(int action)
+{
+	switch (action) {
+	case TCA_EGRESS_REDIR:
+	case TCA_EGRESS_MIRROR:
+		return AT_EGRESS;
+	case TCA_INGRESS_REDIR:
+	case TCA_INGRESS_MIRROR:
+		return AT_INGRESS;
+	default:
+		BUG();
+	}
+}
+
 static void tcf_mirred_release(struct tc_action *a, int bind)
 {
 	struct tcf_mirred *m = to_mirred(a);
@@ -97,6 +116,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 	switch (parm->eaction) {
 	case TCA_EGRESS_MIRROR:
 	case TCA_EGRESS_REDIR:
+	case TCA_INGRESS_REDIR:
+	case TCA_INGRESS_MIRROR:
 		break;
 	default:
 		if (exists)
@@ -156,15 +177,20 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 		      struct tcf_result *res)
 {
 	struct tcf_mirred *m = to_mirred(a);
+	bool m_mac_header_xmit;
 	struct net_device *dev;
 	struct sk_buff *skb2;
-	int retval, err;
+	int retval, err = 0;
+	int m_eaction;
+	int mac_len;
 	u32 at;
 
 	tcf_lastuse_update(&m->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
 
 	rcu_read_lock();
+	m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);
+	m_eaction = READ_ONCE(m->tcfm_eaction);
 	retval = READ_ONCE(m->tcf_action);
 	dev = rcu_dereference(m->tcfm_dev);
 	if (unlikely(!dev)) {
@@ -183,23 +209,36 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 	if (!skb2)
 		goto out;
 
-	if (!(at & AT_EGRESS)) {
-		if (m->tcfm_mac_header_xmit)
+	/* If action's target direction differs than filter's direction,
+	 * and devices expect a mac header on xmit, then mac push/pull is
+	 * needed.
+	 */
+	if (at != tcf_mirred_act_direction(m_eaction) && m_mac_header_xmit) {
+		if (at & AT_EGRESS) {
+			/* caught at egress, act ingress: pull mac */
+			mac_len = skb_network_header(skb) - skb_mac_header(skb);
+			skb_pull_rcsum(skb2, mac_len);
+		} else {
+			/* caught at ingress, act egress: push mac */
 			skb_push_rcsum(skb2, skb->mac_len);
+		}
 	}
 
 	/* mirror is always swallowed */
-	if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+	if (tcf_mirred_is_act_redirect(m_eaction))
 		skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
 
 	skb2->skb_iif = skb->dev->ifindex;
 	skb2->dev = dev;
-	err = dev_queue_xmit(skb2);
+	if (tcf_mirred_act_direction(m_eaction) & AT_EGRESS)
+		err = dev_queue_xmit(skb2);
+	else
+		err = netif_receive_skb(skb2);
 
 	if (err) {
 out:
 		qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
-		if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+		if (tcf_mirred_is_act_redirect(m_eaction))
 			retval = TC_ACT_SHOT;
 	}
 	rcu_read_unlock();
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit and make it a bool
From: Shmulik Ladkani @ 2016-10-13  6:06 UTC (permalink / raw)
  To: David Miller
  Cc: Jamal Hadi Salim, Eric Dumazet, WANG Cong, Daniel Borkmann,
	netdev, Shmulik Ladkani
In-Reply-To: <1476338804-25440-1-git-send-email-shmulik.ladkani@gmail.com>

'tcfm_ok_push' specifies whether a mac_len sized push is needed upon
egress to the target device (if action is performed at ingress).

Rename it to 'tcfm_mac_header_xmit' as this is actually an attribute of
the target device (and use a bool instead of int).

This allows to decouple the attribute from the action to be taken.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
 v2: declare tcfm_mac_header_xmit as bool instead of int

 include/net/tc_act/tc_mirred.h |  2 +-
 net/sched/act_mirred.c         | 11 ++++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 62770ad..9543109 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -8,7 +8,7 @@ struct tcf_mirred {
 	struct tc_action	common;
 	int			tcfm_eaction;
 	int			tcfm_ifindex;
-	int			tcfm_ok_push;
+	bool			tcfm_mac_header_xmit;
 	struct net_device __rcu	*tcfm_dev;
 	struct list_head	tcfm_list;
 };
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 667dc38..16e17a8 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -60,11 +60,12 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 {
 	struct tc_action_net *tn = net_generic(net, mirred_net_id);
 	struct nlattr *tb[TCA_MIRRED_MAX + 1];
+	bool mac_header_xmit = false;
 	struct tc_mirred *parm;
 	struct tcf_mirred *m;
 	struct net_device *dev;
-	int ret, ok_push = 0;
 	bool exists = false;
+	int ret;
 
 	if (nla == NULL)
 		return -EINVAL;
@@ -102,10 +103,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 		case ARPHRD_IPGRE:
 		case ARPHRD_VOID:
 		case ARPHRD_NONE:
-			ok_push = 0;
+			mac_header_xmit = false;
 			break;
 		default:
-			ok_push = 1;
+			mac_header_xmit = true;
 			break;
 		}
 	} else {
@@ -136,7 +137,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 			dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
 		dev_hold(dev);
 		rcu_assign_pointer(m->tcfm_dev, dev);
-		m->tcfm_ok_push = ok_push;
+		m->tcfm_mac_header_xmit = mac_header_xmit;
 	}
 
 	if (ret == ACT_P_CREATED) {
@@ -181,7 +182,7 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 		goto out;
 
 	if (!(at & AT_EGRESS)) {
-		if (m->tcfm_ok_push)
+		if (m->tcfm_mac_header_xmit)
 			skb_push_rcsum(skb2, skb->mac_len);
 	}
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 net-next 3/4] net/sched: tc_mirred: Rename public predicates 'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
From: Shmulik Ladkani @ 2016-10-13  6:06 UTC (permalink / raw)
  To: David Miller
  Cc: Jamal Hadi Salim, Eric Dumazet, WANG Cong, Daniel Borkmann,
	netdev, Shmulik Ladkani, Hariprasad S, Jeff Kirsher,
	Saeed Mahameed, Jiri Pirko, Ido Schimmel, Jakub Kicinski
In-Reply-To: <1476338804-25440-1-git-send-email-shmulik.ladkani@gmail.com>

These accessors are used in various drivers that support tc offloading,
to detect properties of a given 'tc_action'.

'is_tcf_mirred_redirect' tests that the action is TCA_EGRESS_REDIR.
'is_tcf_mirred_mirror' tests that the action is TCA_EGRESS_MIRROR.

As a prep towards supporting INGRESS redir/mirror, rename these
predicates to reflect their true meaning:
  s/is_tcf_mirred_redirect/is_tcf_mirred_egress_redirect/
  s/is_tcf_mirred_mirror/is_tcf_mirred_egress_mirror/

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Hariprasad S <hariprasad@chelsio.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Ido Schimmel <idosch@mellanox.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c    | 2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c        | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c      | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c       | 4 +++-
 drivers/net/ethernet/netronome/nfp/nfp_net_offload.c | 2 +-
 include/net/tc_act/tc_mirred.h                       | 4 ++--
 6 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
index 49d2deb..52af62e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
@@ -113,7 +113,7 @@ static int fill_action_fields(struct adapter *adap,
 		}
 
 		/* Re-direct to specified port in hardware. */
-		if (is_tcf_mirred_redirect(a)) {
+		if (is_tcf_mirred_egress_redirect(a)) {
 			struct net_device *n_dev;
 			unsigned int i, index;
 			bool found = false;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a244d9a..784b0b9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8410,7 +8410,7 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter,
 		}
 
 		/* Redirect to a VF or a offloaded macvlan */
-		if (is_tcf_mirred_redirect(a)) {
+		if (is_tcf_mirred_egress_redirect(a)) {
 			int ifindex = tcf_mirred_ifindex(a);
 
 			err = handle_redirect_action(adapter, ifindex, queue,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index ce8c54d..135a95b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -404,7 +404,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
 			continue;
 		}
 
-		if (is_tcf_mirred_redirect(a)) {
+		if (is_tcf_mirred_egress_redirect(a)) {
 			int ifindex = tcf_mirred_ifindex(a);
 			struct net_device *out_dev;
 			struct mlx5e_priv *out_priv;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 1ec0a4c..43a5edd 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1237,8 +1237,10 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	tcf_exts_to_list(cls->exts, &actions);
 	list_for_each_entry(a, &actions, list) {
-		if (!is_tcf_mirred_mirror(a) || protocol != htons(ETH_P_ALL))
+		if (!is_tcf_mirred_egress_mirror(a) ||
+		    protocol != htons(ETH_P_ALL)) {
 			return -ENOTSUPP;
+		}
 
 		err = mlxsw_sp_port_add_cls_matchall_mirror(mlxsw_sp_port, cls,
 							    a, ingress);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
index 8acfb63..cfed40c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
@@ -128,7 +128,7 @@ static void nfp_net_bpf_stats_reset(struct nfp_net *nn)
 		if (is_tcf_gact_shot(a))
 			return NN_ACT_TC_DROP;
 
-		if (is_tcf_mirred_redirect(a) &&
+		if (is_tcf_mirred_egress_redirect(a) &&
 		    tcf_mirred_ifindex(a) == nn->netdev->ifindex)
 			return NN_ACT_TC_REDIR;
 	}
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 9543109..604bc31 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -14,7 +14,7 @@ struct tcf_mirred {
 };
 #define to_mirred(a) ((struct tcf_mirred *)a)
 
-static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_redirect(const struct tc_action *a)
 {
 #ifdef CONFIG_NET_CLS_ACT
 	if (a->ops && a->ops->type == TCA_ACT_MIRRED)
@@ -23,7 +23,7 @@ static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
 	return false;
 }
 
-static inline bool is_tcf_mirred_mirror(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_mirror(const struct tc_action *a)
 {
 #ifdef CONFIG_NET_CLS_ACT
 	if (a->ops && a->ops->type == TCA_ACT_MIRRED)
-- 
1.9.1

^ permalink raw reply related

* Re: Kernel 4.6.7-rt13: Intel Ethernet driver igb causes huge latencies in cyclictest
From: Koehrer Mathias (ETAS/ESW5) @ 2016-10-13  6:15 UTC (permalink / raw)
  To: Julia Cartwright
  Cc: Williams, Mitch A, Kirsher, Jeffrey T, Greg,
	netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
	linux-rt-users@vger.kernel.org, Sebastian Andrzej Siewior
In-Reply-To: <20161010193958.GE22235@jcartwri.amer.corp.natinst.com>

Hi Julia,

thanks for the detailed analysis!
> 
> [...] 
> Okay, we finally received our wakeup event.  We were expecting to be woken up at
> 10024735653388ns, but were actually woken up at 10024735682387ns.
> 
>   10024735682387 - 10024735653388 = 28999ns
> 
> Our timer fired ~29us late!  But why...?
> 
> Sorry I don't have answers, just more questions.  I do wonder what
> kworker/0:3 was up to at the time the timer interrupt should have fired.
> 
>    Julia
I have now instrumented the igb driver to generate trace points that allows me to identify in 
more details what's going on.
This delivered interesting results!

Here are some places where I added traces:
In file igb_ptp.c:
void igb_ptp_rx_hang(struct igb_adapter *adapter)
{
        struct e1000_hw *hw = &adapter->hw;
        unsigned long rx_event;
        u32 tsyncrxctl;
        trace_igb(700);
        tsyncrxctl = rd32(E1000_TSYNCRXCTL);
        trace_igb(701);

        /* Other hardware uses per-packet timestamps */
        if (hw->mac.type != e1000_82576)
                return;
...

In file igb_main.c:
static void igb_check_lvmmc(struct igb_adapter *adapter)
{
        struct e1000_hw *hw = &adapter->hw;
        u32 lvmmc;

        trace_igb(600);
        lvmmc = rd32(E1000_LVMMC);
        trace_igb(601);
        if (lvmmc) {
...

When I run now my test, I get the following trace:
[...]
kworker/-607     0....... 107315621us+: igb: val: 700
kworker/-607     0d..h... 107315640us : irq_handler_entry: irq=47 name=eth2-rx-0
kworker/-607     0d..h... 107315640us : irq_handler_exit: irq=47 ret=handled
kworker/-607     0d..h1.. 107315640us : sched_waking: comm=irq/47-eth2-rx- pid=18009 prio=49 target_cpu=000
kworker/-607     0dN.h2.. 107315641us : sched_wakeup: comm=irq/47-eth2-rx- pid=18009 prio=49 target_cpu=000
kworker/-607     0dN.h1.. 107315643us : irq_handler_entry: irq=48 name=eth2-tx-0
kworker/-607     0dN.h1.. 107315643us : irq_handler_exit: irq=48 ret=handled
kworker/-607     0dN.h2.. 107315643us : sched_waking: comm=irq/48-eth2-tx- pid=18010 prio=49 target_cpu=000
kworker/-607     0dN.h3.. 107315644us : sched_wakeup: comm=irq/48-eth2-tx- pid=18010 prio=49 target_cpu=000
kworker/-607     0dN..1.. 107315644us : rcu_utilization: Start context switch
kworker/-607     0dN..1.. 107315644us : rcu_utilization: End context switch
kworker/-607     0dN..2.. 107315644us : sched_stat_runtime: comm=kworker/0:1 pid=607 runtime=88996 [ns] vruntime=49754678074 [ns]
kworker/-607     0d...2.. 107315645us : sched_switch: prev_comm=kworker/0:1 prev_pid=607 prev_prio=120 prev_state=R+ ==> next_comm=irq/47-eth2-rx- next_pid=18009 next_prio=49
irq/47-e-18009   0d....11 107315646us : softirq_raise: vec=3 [action=NET_RX]
irq/47-e-18009   0.....12 107315646us : softirq_entry: vec=3 [action=NET_RX]
irq/47-e-18009   0.....12 107315647us : napi_poll: napi poll on napi struct ffff88040ae58c50 for device eth2 work 0 budget 64
irq/47-e-18009   0.....12 107315647us : softirq_exit: vec=3 [action=NET_RX]
irq/47-e-18009   0d...1.. 107315648us : rcu_utilization: Start context switch
irq/47-e-18009   0d...1.. 107315648us : rcu_utilization: End context switch
irq/47-e-18009   0d...2.. 107315648us : sched_switch: prev_comm=irq/47-eth2-rx- prev_pid=18009 prev_prio=49 prev_state=S ==> next_comm=irq/48-eth2-tx- next_pid=18010 next_prio=49
irq/48-e-18010   0d....11 107315649us : softirq_raise: vec=3 [action=NET_RX]
irq/48-e-18010   0.....12 107315649us : softirq_entry: vec=3 [action=NET_RX]
irq/48-e-18010   0.....12 107315650us : napi_poll: napi poll on napi struct ffff88040ae5f450 for device eth2 work 0 budget 64
irq/48-e-18010   0.....12 107315650us : softirq_exit: vec=3 [action=NET_RX]
irq/48-e-18010   0d...1.. 107315651us : rcu_utilization: Start context switch
irq/48-e-18010   0d...1.. 107315651us : rcu_utilization: End context switch
irq/48-e-18010   0d...2.. 107315651us : sched_switch: prev_comm=irq/48-eth2-tx- prev_pid=18010 prev_prio=49 prev_state=S ==> next_comm=kworker/0:1 next_pid=607 next_prio=120
kworker/-607     0....... 107315652us : igb: val: 701
kworker/-607     0....... 107315652us : igb: val: 106
kworker/-607     0....... 107315652us : igb: val: 107
kworker/-607     0....... 107315652us+: igb: val: 600
kworker/-607     0d..h... 107315689us : local_timer_entry: vector=239
kworker/-607     0d..h1.. 107315689us : hrtimer_interrupt: cpu=0 offset=-34521 curr=kworker/0:1[120] thread=cyclictest[19]
kworker/-607     0d..h1.. 107315689us : hrtimer_cancel: hrtimer=ffff8803d42efe18
kworker/-607     0d..h... 107315689us : hrtimer_expire_entry: hrtimer=ffff8803d42efe18 function=hrtimer_wakeup now=752735681960
kworker/-607     0d..h1.. 107315689us : sched_waking: comm=cyclictest pid=18015 prio=19 target_cpu=000
kworker/-607     0dN.h2.. 107315690us : sched_wakeup: comm=cyclictest pid=18015 prio=19 target_cpu=000
kworker/-607     0dN.h... 107315690us : hrtimer_expire_exit: hrtimer=ffff8803d42efe18
kworker/-607     0dN.h1.. 107315690us : hrtimer_interrupt: cpu=0 offset=318040 curr=kworker/0:1[120] thread=<none>[-1]
kworker/-607     0dN.h... 107315690us : write_msr: 6e0, value 28096cdb9ce
kworker/-607     0dN.h... 107315690us : local_timer_exit: vector=239
kworker/-607     0dN..1.. 107315690us : rcu_utilization: Start context switch
kworker/-607     0dN..1.. 107315691us : rcu_utilization: End context switch
kworker/-607     0dN..2.. 107315691us : sched_stat_runtime: comm=kworker/0:1 pid=607 runtime=38439 [ns] vruntime=49754716513 [ns]
kworker/-607     0d...2.. 107315691us : sched_switch: prev_comm=kworker/0:1 prev_pid=607 prev_prio=120 prev_state=R+ ==> next_comm=cyclictest next_pid=18015 next_prio=19
kworker/-607     0d...2.. 107315691us : x86_fpu_regs_activated: x86/fpu: ffff8803f7f55940 fpregs_active: 1 fpstate_active: 1 counter: 99 xfeatures: 2 xcomp_bv: 0
kworker/-607     0d...2.. 107315691us : write_msr: c0000100, value 7ffff7400700
cyclicte-18015   0....... 107315692us : sys_exit: NR 230 = 0
cyclicte-18015   0....... 107315697us : sys_enter: NR 1 (5, 7ffff7400300, 1f, 7ffff77a5460, 2, 7ffff744c99a)
cyclicte-18015   0.....11 107315698us : tracing_mark_write: hit latency threshold (37 > 33)
cyclicte-18015   0....... 107315699us : sys_exit: NR 1 = 31
cyclicte-18015   0....... 107315699us : sys_enter: NR 1 (4, 4076b0, 2, 7ffff77a5460, 2, 7ffff744c99a)


Very interesting is also the trace that I get in an idle system - without cyclictest running.
When I just enable my igb tracepoint I got the following result:
[...]
kworker/-607     0....... 585779012us+: igb: val: 700
kworker/-607     0....... 585779042us : igb: val: 701
kworker/-607     0....... 585779042us : igb: val: 106
kworker/-607     0....... 585779043us : igb: val: 107
kworker/-607     0....... 585779043us+: igb: val: 600
kworker/-607     0....... 585779080us : igb: val: 601

The time between my trace points 700 and 701 is about 30us, 
the time between my trace points 600 and 601 is even 37us!!
The code in between is 
     tsyncrxctl = rd32(E1000_TSYNCRXCTL);
resp.
     lvmmc = rd32(E1000_LVMMC);   

In both cases this is a single read from a register.
I have no idea why this single read could take that much time!
Is it possible that the igb hardware is in a state that delays the read access
and this is why the whole I/O system might be delayed?

I hope this delivers more details on understanding this issue.

Thanks for any feedback!

Regards

Mathias

^ permalink raw reply

* Re: slab corruption with current -git
From: Markus Trippelsdorf @ 2016-10-13  6:27 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Lameter, Jens Axboe, linux-kernel, aconole,
	David Miller, pablo, linux-fsdevel, viro, netfilter-devel, netdev,
	akpm, fw, tytso
In-Reply-To: <CA+55aFwsUR4-YmOYgJOOO4a2e48M4_tk7YhAo4s5KZQQxUjpZw@mail.gmail.com>

On 2016.10.12 at 23:18 -0700, Linus Torvalds wrote:
> On Oct 12, 2016 23:07, "Markus Trippelsdorf" <markus@trippelsdorf.de> wrote:
> >
> > This is nf_register_net_hook at net/netfilter/core.c:106
>
> The "*regs" access?

Yeah.

105         entry->orig_ops = reg;
106         entry->ops      = *reg;
107         entry->next     = NULL;

^ permalink raw reply

* Re: [PATCH v6] net: ip, diag -- Add diag interface for raw sockets
From: Cyrill Gorcunov @ 2016-10-13  7:16 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, eric.dumazet, jhs, dsa, linux-kernel, kuznet, jmorris,
	yoshfuji, kaber, avagin, stephen
In-Reply-To: <20161012.195504.658773172040030000.davem@davemloft.net>

On Wed, Oct 12, 2016 at 07:55:04PM -0400, David Miller wrote:
> From: Cyrill Gorcunov <gorcunov@gmail.com>
> Date: Wed, 12 Oct 2016 09:53:29 +0300
> 
> > I can't rename the field, neither a can use union.
> 
> Remind me again what is wrong with using an anonymous union?

Anon union would be a preferred but Eric pointed me that even
though it might cause problems (https://patchwork.kernel.org/patch/9353365/)

 | Note that some programs could fail to compile with the added union
 | anyway.
 |
 | Some gcc versions are unable to compile a static init with an union
 |
 | struct inet_diag_req_v2 foo = { .pad = 0, sdiag_family = AF_INET, };
 |
 | When I cooked my recent fq commit I simply removed a pad and replaced
 | it :
 |
 | git show fefa569a9d4bc4 -- include

^ permalink raw reply

* Re: [PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
From: Jiri Pirko @ 2016-10-13  7:30 UTC (permalink / raw)
  To: David Ahern
  Cc: jiri, netdev, davem, dledford, sean.hefty, hal.rosenstock,
	linux-rdma, j.vosburgh, vfalico, andy, jeffrey.t.kirsher,
	intel-wired-lan
In-Reply-To: <1476305519-28833-3-git-send-email-dsa@cumulusnetworks.com>

Wed, Oct 12, 2016 at 10:51:50PM CEST, dsa@cumulusnetworks.com wrote:
>This patch introduces netdev_walk_all_upper_dev_rcu,
>netdev_walk_all_lower_dev and netdev_walk_all_lower_dev_rcu. These
>functions recursively walk the adj_list of devices to determine all upper
>and lower devices.
>
>The functions take a callback function that is invoked for each device
>in the list. If the callback returns non-0, the walk is terminated and
>the functions return that code back to callers.
>
>Signed-off-by: David Ahern <dsa@cumulusnetworks.com>

[...]

>+int netdev_walk_all_lower_dev(struct net_device *dev,
>+			      int (*fn)(struct net_device *dev,
>+					void *data),
>+			      void *data)
>+{
>+	struct list_head *iter;
>+	struct net_device *ldev;
>+	int ret;
>+
>+	for (iter = &(dev)->adj_list.lower,
>+	     ldev = netdev_next_lower_dev(dev, &(iter));
>+	     ldev;
>+	     ldev = netdev_next_lower_dev(dev, &(iter))) {
>+		/* first is the lower device itself */
>+		ret = fn(ldev, data);
>+		if (ret)
>+			return ret;
>+
>+		/* then look at all of its lower devices */
>+		ret = netdev_walk_all_lower_dev(ldev, fn, data);

I believe that Veaceslav's reason to collapse the upper/lower trees was
to avoid this recursivity. I also believe that the recursivity was a big
issue for DaveM and BenH. Something changed?

^ permalink raw reply

* Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags
From: Christian König @ 2016-10-13  7:32 UTC (permalink / raw)
  To: Lorenzo Stoakes, linux-mm
  Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
	dri-devel, sparclinux, linux-ia64, linux-s390, linux-samsung-soc,
	linux-scsi, linux-rdma, x86, Hugh Dickins, linux-media,
	Rik van Riel, intel-gfx, adi-buildroot-devel, ceph-devel,
	linux-arm-kernel, Mel Gorman, linux-cris-kernel, netdev,
	linuxppc-dev, linux-kernel, linux-security-module, linux-alpha,
	linux-fsdevel
In-Reply-To: <20161013002020.3062-1-lstoakes@gmail.com>

Am 13.10.2016 um 02:20 schrieb Lorenzo Stoakes:
> This patch series adjusts functions in the get_user_pages* family such that
> desired FOLL_* flags are passed as an argument rather than implied by flags.
>
> The purpose of this change is to make the use of FOLL_FORCE explicit so it is
> easier to grep for and clearer to callers that this flag is being used. The use
> of FOLL_FORCE is an issue as it overrides missing VM_READ/VM_WRITE flags for the
> VMA whose pages we are reading from/writing to, which can result in surprising
> behaviour.
>
> The patch series came out of the discussion around commit 38e0885, which
> addressed a BUG_ON() being triggered when a page was faulted in with PROT_NONE
> set but having been overridden by FOLL_FORCE. do_numa_page() was run on the
> assumption the page _must_ be one marked for NUMA node migration as an actual
> PROT_NONE page would have been dealt with prior to this code path, however
> FOLL_FORCE introduced a situation where this assumption did not hold.
>
> See https://marc.info/?l=linux-mm&m=147585445805166 for the patch proposal.
>
> Lorenzo Stoakes (10):
>    mm: remove write/force parameters from __get_user_pages_locked()
>    mm: remove write/force parameters from __get_user_pages_unlocked()
>    mm: replace get_user_pages_unlocked() write/force parameters with gup_flags
>    mm: replace get_user_pages_locked() write/force parameters with gup_flags
>    mm: replace get_vaddr_frames() write/force parameters with gup_flags
>    mm: replace get_user_pages() write/force parameters with gup_flags
>    mm: replace get_user_pages_remote() write/force parameters with gup_flags
>    mm: replace __access_remote_vm() write parameter with gup_flags
>    mm: replace access_remote_vm() write parameter with gup_flags
>    mm: replace access_process_vm() write parameter with gup_flags

Patch number 6 in this series (which touches drivers I co-maintain) is 
Acked-by: Christian König <christian.koenig@amd.com>.

In general looks like a very nice cleanup to me, but I'm not enlightened 
enough to full judge.

Regards,
Christian.

>
>   arch/alpha/kernel/ptrace.c                         |  9 ++--
>   arch/blackfin/kernel/ptrace.c                      |  5 ++-
>   arch/cris/arch-v32/drivers/cryptocop.c             |  4 +-
>   arch/cris/arch-v32/kernel/ptrace.c                 |  4 +-
>   arch/ia64/kernel/err_inject.c                      |  2 +-
>   arch/ia64/kernel/ptrace.c                          | 14 +++---
>   arch/m32r/kernel/ptrace.c                          | 15 ++++---
>   arch/mips/kernel/ptrace32.c                        |  5 ++-
>   arch/mips/mm/gup.c                                 |  2 +-
>   arch/powerpc/kernel/ptrace32.c                     |  5 ++-
>   arch/s390/mm/gup.c                                 |  3 +-
>   arch/score/kernel/ptrace.c                         | 10 +++--
>   arch/sh/mm/gup.c                                   |  3 +-
>   arch/sparc/kernel/ptrace_64.c                      | 24 +++++++----
>   arch/sparc/mm/gup.c                                |  3 +-
>   arch/x86/kernel/step.c                             |  3 +-
>   arch/x86/mm/gup.c                                  |  2 +-
>   arch/x86/mm/mpx.c                                  |  5 +--
>   arch/x86/um/ptrace_32.c                            |  3 +-
>   arch/x86/um/ptrace_64.c                            |  3 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c            |  7 ++-
>   drivers/gpu/drm/etnaviv/etnaviv_gem.c              |  7 ++-
>   drivers/gpu/drm/exynos/exynos_drm_g2d.c            |  3 +-
>   drivers/gpu/drm/i915/i915_gem_userptr.c            |  6 ++-
>   drivers/gpu/drm/radeon/radeon_ttm.c                |  3 +-
>   drivers/gpu/drm/via/via_dmablit.c                  |  4 +-
>   drivers/infiniband/core/umem.c                     |  6 ++-
>   drivers/infiniband/core/umem_odp.c                 |  7 ++-
>   drivers/infiniband/hw/mthca/mthca_memfree.c        |  2 +-
>   drivers/infiniband/hw/qib/qib_user_pages.c         |  3 +-
>   drivers/infiniband/hw/usnic/usnic_uiom.c           |  5 ++-
>   drivers/media/pci/ivtv/ivtv-udma.c                 |  4 +-
>   drivers/media/pci/ivtv/ivtv-yuv.c                  |  5 ++-
>   drivers/media/platform/omap/omap_vout.c            |  2 +-
>   drivers/media/v4l2-core/videobuf-dma-sg.c          |  7 ++-
>   drivers/media/v4l2-core/videobuf2-memops.c         |  6 ++-
>   drivers/misc/mic/scif/scif_rma.c                   |  3 +-
>   drivers/misc/sgi-gru/grufault.c                    |  2 +-
>   drivers/platform/goldfish/goldfish_pipe.c          |  3 +-
>   drivers/rapidio/devices/rio_mport_cdev.c           |  3 +-
>   drivers/scsi/st.c                                  |  5 +--
>   .../interface/vchiq_arm/vchiq_2835_arm.c           |  3 +-
>   .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  3 +-
>   drivers/video/fbdev/pvr2fb.c                       |  4 +-
>   drivers/virt/fsl_hypervisor.c                      |  4 +-
>   fs/exec.c                                          |  9 +++-
>   fs/proc/base.c                                     | 19 +++++---
>   include/linux/mm.h                                 | 18 ++++----
>   kernel/events/uprobes.c                            |  6 ++-
>   kernel/ptrace.c                                    | 16 ++++---
>   mm/frame_vector.c                                  |  9 ++--
>   mm/gup.c                                           | 50 ++++++++++------------
>   mm/memory.c                                        | 16 ++++---
>   mm/mempolicy.c                                     |  2 +-
>   mm/nommu.c                                         | 38 +++++++---------
>   mm/process_vm_access.c                             |  7 ++-
>   mm/util.c                                          |  8 ++--
>   net/ceph/pagevec.c                                 |  2 +-
>   security/tomoyo/domain.c                           |  2 +-
>   virt/kvm/async_pf.c                                |  3 +-
>   virt/kvm/kvm_main.c                                | 11 +++--
>   61 files changed, 260 insertions(+), 187 deletions(-)
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH net-next 00/11] net: Fix netdev adjacency tracking
From: Jiri Pirko @ 2016-10-13  7:34 UTC (permalink / raw)
  To: David Ahern
  Cc: jiri, netdev, davem, dledford, sean.hefty, hal.rosenstock,
	linux-rdma, j.vosburgh, vfalico, andy, jeffrey.t.kirsher,
	intel-wired-lan
In-Reply-To: <1476305519-28833-1-git-send-email-dsa@cumulusnetworks.com>

Wed, Oct 12, 2016 at 10:51:48PM CEST, dsa@cumulusnetworks.com wrote:
>The netdev adjacency tracking is failing to create proper dependencies
>for some topologies. For example this topology
>
>        +--------+
>        |  myvrf |
>        +--------+
>          |    |
>          |  +---------+
>          |  | macvlan |
>          |  +---------+
>          |    |
>      +----------+
>      |  bridge  |
>      +----------+
>          |
>      +--------+
>      | bond0  |
>      +--------+
>          |
>      +--------+
>      |  eth3  |
>      +--------+
>
>hits 1 of 2 problems depending on the order of enslavement. The base set of
>commands for both cases:
>
>    ip link add bond1 type bond
>    ip link set bond1 up
>    ip link set eth3 down
>    ip link set eth3 master bond1
>    ip link set eth3 up
>
>    ip link add bridge type bridge
>    ip link set bridge up
>    ip link add macvlan link bridge type macvlan
>    ip link set macvlan up
>
>    ip link add myvrf type vrf table 1234
>    ip link set myvrf up
>
>    ip link set bridge master myvrf
>
>Case 1 enslave macvlan to the vrf before enslaving the bond to the bridge:
>
>    ip link set macvlan master myvrf
>    ip link set bond1 master bridge
>
>Attempts to delete the VRF:
>    ip link delete myvrf
>
>trigger the BUG in __netdev_adjacent_dev_remove:
>
>[  587.405260] tried to remove device eth3 from myvrf
>[  587.407269] ------------[ cut here ]------------
>[  587.408918] kernel BUG at /home/dsa/kernel.git/net/core/dev.c:5661!
>[  587.411113] invalid opcode: 0000 [#1] SMP
>[  587.412454] Modules linked in: macvlan bridge stp llc bonding vrf
>[  587.414765] CPU: 0 PID: 726 Comm: ip Not tainted 4.8.0+ #109
>[  587.416766] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
>[  587.420241] task: ffff88013ab6eec0 task.stack: ffffc90000628000
>[  587.422163] RIP: 0010:[<ffffffff813cef03>]  [<ffffffff813cef03>] __netdev_adjacent_dev_remove+0x40/0x12c
>...
>[  587.446053] Call Trace:
>[  587.446424]  [<ffffffff813d1542>] __netdev_adjacent_dev_unlink+0x20/0x3c
>[  587.447390]  [<ffffffff813d16a3>] netdev_upper_dev_unlink+0xfa/0x15e
>[  587.448297]  [<ffffffffa00003a3>] vrf_del_slave+0x13/0x2a [vrf]
>[  587.449153]  [<ffffffffa00004a4>] vrf_dev_uninit+0xea/0x114 [vrf]
>[  587.450036]  [<ffffffff813d19b0>] rollback_registered_many+0x22b/0x2da
>[  587.450974]  [<ffffffff813d1aac>] unregister_netdevice_many+0x17/0x48
>[  587.451903]  [<ffffffff813de444>] rtnl_delete_link+0x3c/0x43
>[  587.452719]  [<ffffffff813dedcd>] rtnl_dellink+0x180/0x194
>
>When the BUG is converted to a WARN_ON it shows 4 missing adjacencies:
>  eth3 - myvrf, mvrf - eth3, bond1 - myvrf and myvrf - bond1
>
>All of those are because the __netdev_upper_dev_link function does not
>properly link macvlan lower devices to myvrf when it is enslaved.
>
>The second case just flips the ordering of the enslavements:
>    ip link set bond1 master bridge
>    ip link set macvlan master myvrf
>
>Then run:
>    ip link delete bond1
>    ip link delete myvrf
>
>The vrf delete command hangs because myvrf has a reference that has not
>been released. In this case the removal code does not account for 2 paths 
>between eth3 and myvrf - one from bridge to vrf and the other through the
>macvlan.
>
>Rather than try to maintain a linked list of all upper and lower devices
>per netdevice, only track the direct neighbors. The remaining stack can
>be determined by recursively walking the neighbors.

Although I didn't like the "all-list" idea when Veaceslav pushed it
because it looked to me like a big hammer, it turned out to be very handy
and quick for traversing neighbours. Why it cannot be fixed?

The walks with possibly hundreds of function calls instead of a single
list traverse worries me.

^ permalink raw reply

* [patch] tipc: info leak in __tipc_nl_add_udp_addr()
From: Dan Carpenter @ 2016-10-13  8:06 UTC (permalink / raw)
  To: Jon Maloy, Richard Alpe
  Cc: Ying Xue, David S. Miller, netdev, tipc-discussion, linux-kernel,
	kernel-janitors

We should clear out the padding and unused struct members so that we
don't expose stack information to userspace.

Fixes: fdb3accc2c15 ('tipc: add the ability to get UDP options via netlink')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This goes into the net tree.

diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index d80cd3f..78cab9c 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -407,6 +407,7 @@ static int __tipc_nl_add_udp_addr(struct sk_buff *skb,
 	if (ntohs(addr->proto) == ETH_P_IP) {
 		struct sockaddr_in ip4;
 
+		memset(&ip4, 0, sizeof(ip4));
 		ip4.sin_family = AF_INET;
 		ip4.sin_port = addr->port;
 		ip4.sin_addr.s_addr = addr->ipv4.s_addr;
@@ -417,6 +418,7 @@ static int __tipc_nl_add_udp_addr(struct sk_buff *skb,
 	} else if (ntohs(addr->proto) == ETH_P_IPV6) {
 		struct sockaddr_in6 ip6;
 
+		memset(&ip6, 0, sizeof(ip6));
 		ip6.sin6_family = AF_INET6;
 		ip6.sin6_port  = addr->port;
 		memcpy(&ip6.sin6_addr, &addr->ipv6, sizeof(struct in6_addr));

^ permalink raw reply related

* Re: [PATCH RFC v2 net-next 0/8] net: qualcomm: add QCA7000 UART driver
From: Stefan Wahren @ 2016-10-13  8:09 UTC (permalink / raw)
  To: Jiri Slaby, Greg Kroah-Hartman, David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <1474888086-514-1-git-send-email-stefan.wahren@i2se.com>


> Stefan Wahren <stefan.wahren@i2se.com> hat am 26. September 2016 um 13:07
> geschrieben:
> 
> 
> The Qualcomm QCA7000 HomePlug GreenPHY supports two interfaces:
> UART and SPI. This patch series adds the missing support for UART.
> 
> This driver based on the Qualcomm code [1], but contains some changes:
> * use random MAC address per default
> * use net_device_stats from device
> * share frame decoding between SPI and UART driver
> * improve error handling
> * reimplement tty_wakeup with work queue (based on slcan)
> 
> The patches 1 - 3 are just for clean up and are not related to
> the UART support. Patches 4 - 7 prepare the existing QCA7000
> code for UART support. The last patch contains the new driver.
> 
> In order to test the driver a modified slattach in userspace is also
> required. A prepared busybox (includes slattach) can be found here [2].
> 
> The code itself has been tested on a Freescale i.MX28 board.
> 
> Changes in v2:
>   * fix build issue by using netif_trans_update() and dev_trans_start()
> 
> [1] - https://github.com/IoE/qca7000
> [2] - https://github.com/lategoodbye/busybox/commits/qca7k_support_1_24
> 

Gently ping, since i didn't receive any comment for this version.

^ permalink raw reply

* Re: [PATCH RFC v2 net-next 0/8] net: qualcomm: add QCA7000 UART driver
From: Greg Kroah-Hartman @ 2016-10-13  8:14 UTC (permalink / raw)
  To: Stefan Wahren; +Cc: Jiri Slaby, David S. Miller, netdev, linux-kernel
In-Reply-To: <1257730210.379562.ef1d8693-0ff3-4b50-a424-49eebba23da1.open-xchange@email.1und1.de>

On Thu, Oct 13, 2016 at 10:09:09AM +0200, Stefan Wahren wrote:
> 
> > Stefan Wahren <stefan.wahren@i2se.com> hat am 26. September 2016 um 13:07
> > geschrieben:
> > 
> > 
> > The Qualcomm QCA7000 HomePlug GreenPHY supports two interfaces:
> > UART and SPI. This patch series adds the missing support for UART.
> > 
> > This driver based on the Qualcomm code [1], but contains some changes:
> > * use random MAC address per default
> > * use net_device_stats from device
> > * share frame decoding between SPI and UART driver
> > * improve error handling
> > * reimplement tty_wakeup with work queue (based on slcan)
> > 
> > The patches 1 - 3 are just for clean up and are not related to
> > the UART support. Patches 4 - 7 prepare the existing QCA7000
> > code for UART support. The last patch contains the new driver.
> > 
> > In order to test the driver a modified slattach in userspace is also
> > required. A prepared busybox (includes slattach) can be found here [2].
> > 
> > The code itself has been tested on a Freescale i.MX28 board.
> > 
> > Changes in v2:
> >   * fix build issue by using netif_trans_update() and dev_trans_start()
> > 
> > [1] - https://github.com/IoE/qca7000
> > [2] - https://github.com/lategoodbye/busybox/commits/qca7k_support_1_24
> > 
> 
> Gently ping, since i didn't receive any comment for this version.

It's the middle of the merge window, I can't add any new patches right
now.  Please wait until after 4.9-rc1 is out.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH net] net_sched: do not broadcast RTM_GETTFILTER result
From: Eric Dumazet @ 2016-10-13  7:46 UTC (permalink / raw)
  To: Cong Wang; +Cc: David Miller, netdev, Jamal Hadi Salim
In-Reply-To: <CAM_iQpWDwwo+DEi2aytoAnTHWKWeTRiMZqXfvaB7=T9eOnvR-w@mail.gmail.com>

On Wed, 2016-10-12 at 09:36 -0700, Cong Wang wrote:
> On Sun, Oct 9, 2016 at 8:25 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > +       if (unicast)
> > +               return netlink_unicast(net->rtnl, skb, portid, MSG_DONTWAIT);
> 
> Nit: rtnl_unicast() is simpler.

I copied code in rtnetlink_send(), I guess we could use rtnl_unicast()
there as well.

^ permalink raw reply

* [patch -next] net: rtnl: info leak in rtnl_fill_vfinfo()
From: Dan Carpenter @ 2016-10-13  8:45 UTC (permalink / raw)
  To: David S. Miller, Moshe Shemesh
  Cc: Roopa Prabhu, Nicolas Dichtel, Nikolay Aleksandrov, Eric Dumazet,
	Hannes Frederic Sowa, Nogah Frankel, Brenden Blanco, netdev,
	kernel-janitors

The "vf_vlan_info" struct ends with a 2 byte struct hole so we have to
memset it to ensure that no stack information is revealed to user space.

Fixes: 79aab093a0b5 ('net: Update API for VF vlan protocol 802.1ad support')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b06d2f4..fb7348f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1144,6 +1144,8 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
 	if (dev->netdev_ops->ndo_get_vf_config(dev, vfs_num, &ivi))
 		return 0;
 
+	memset(&vf_vlan_info, 0, sizeof(vf_vlan_info));
+
 	vf_mac.vf =
 		vf_vlan.vf =
 		vf_vlan_info.vf =

^ permalink raw reply related

* Re: [patch net-next RFC 0/6] Add support for offloading packet-sampling
From: Jiri Pirko @ 2016-10-13  8:48 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: netdev, davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
	geert+renesas, stephen, xiyou.wangcong, linux
In-Reply-To: <57FF37F5.4070100@cumulusnetworks.com>

Thu, Oct 13, 2016 at 09:29:57AM CEST, roopa@cumulusnetworks.com wrote:
>On 10/12/16, 5:41 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Add the sample tc action, which allows to sample packet matching
>> a classifier. The sample action peeks randomly packets, duplicates them,
>> truncates them and adds informative metadata on the packet, for example,
>> the input interface and the original packet length. The sampled packets
>> are marked to allow matching them and redirecting them to a specific
>> collector device.
>>
>> The sampled packets metadata is packed using ife encapsulation. To do
>> that, this patch-set extracts ife logics from the tc_ife action into an
>> independent ife module, and uses that functionality to pack the metadata.
>> To include all the needed metadata, this patch-set introduces some new
>> IFE_META tlv types.
>>
>> In addition, Add the support for offloading the matchall-sample tc command
>> in the Mellanox mlxsw driver, for ingress qdiscs.
>>
>> Yotam Gigi (6):
>>   Introduce ife encapsulation module
>>   act_ife: Change to use ife module
>>   ife: Introduce new metadata tlv types
>>   Introduce sample tc action
>>   mlxsw: reg: add the Monitoring Packet Sampling Configuration Register
>>   mlxsw: packet sample: Add packet sample offloading support
>>
>
>we spoke with yotam about this at netdev1.2. and also remember speaking about this on our switchdev calls:
>Today our driver uses NFLOG to log packets to a netlink socket and hsflowd supported by the sflow
>people (at http://www.sflow.net/) is capable of reading from a nflog socket. NFLOG has the required netlink
>attribute markers for packet header/data (which we can possibly extend). We could also add nflog like action
>in tc if needed.
>
>sflow agents like hsflowd are capable of sending packets to an external collector with the required sflow header.
>Instead of re-inventing a new API for sflow, would be better to standardize/unify on existing mechanisms.
>
>Also, this patch series requires a new device to be created which can be avoided if we used
>existing mechanisms like NFLOG.

When I was first thinking about re-using NFLOG, it seemed like an
abusal. We need to call it from driver directly, which sounds odd.
However, since we use sample_packet_pack function to wrap it up, the
NFLOG is called from the tc action code, it does not look bad.
Yet still, this has nothing in common with netfilter, only using it's
log facilities. That is odd.

I think that the IFE ways is way more clear and generic and not-abusing.
However you are right the NFLOG way has advantage of existing user
component. I'm not sure how to do this :(

^ permalink raw reply

* [patch] liquidio: CN23XX: fix a loop timeout
From: Dan Carpenter @ 2016-10-13  8:56 UTC (permalink / raw)
  To: Derek Chickles, Raghu Vatsavayi
  Cc: Satanand Burla, Felix Manlunas, netdev, kernel-janitors

This is supposed to loop 1000 times and then give up.  The problem is
it's a post-op and after the loop we test if "loop" is zero when really
it would be -1.  Fix this by making it a pre-op.

Fixes: 1b7c55c4538b ("liquidio: CN23XX queue manipulation")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
index bddb198..380a641 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
@@ -693,7 +693,7 @@ static int cn23xx_enable_io_queues(struct octeon_device *oct)
 				while ((reg_val & CN23XX_PKT_INPUT_CTL_RST) &&
 				       !(reg_val &
 					 CN23XX_PKT_INPUT_CTL_QUIET) &&
-				       loop--) {
+				       --loop) {
 					reg_val = octeon_read_csr64(
 					    oct,
 					    CN23XX_SLI_IQ_PKT_CONTROL64(q_no));

^ permalink raw reply related

* Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
From: Luca Coelho @ 2016-10-13  9:01 UTC (permalink / raw)
  To: Chris Rorvick, Paul Bolle
  Cc: Intel Linux Wireless, Emmanuel Grumbach, Johannes Berg,
	Kalle Valo, Oren Givon, linux-wireless, netdev, linux-kernel
In-Reply-To: <CAEUsAPYYJ3Gmh0T16veCn3wnzdD8bTxE+_U-AUYQpMo3TUd4Mg@mail.gmail.com>

On Wed, 2016-10-12 at 12:50 -0500, Chris Rorvick wrote:
> Hi Luca,
> 
> FYI, It seems that Google does not like your email as I'm not
> receiving any of your messages in gmail.  Some responses below:

That's odd.  It works for me.  Replied privately to try to sort this
out.


> On Wed, 2016-10-12 at 15:24 +0300, Luca Coelho wrote:
> > Hi Chris,
> > On Tue, 2016-10-11 at 09:09 -0500, Chris Rorvick wrote:
> > > On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle [at] tiscali> wrote:
> > > > > This is not coming from the NIC itself, but from the platform's ACPI
> > > > > tables. Can you tell us which platform you are using?
> > > 
> > > 
> > > Interesting. I'm running a Dell XPS 13 9350. I replaced the
> > > factory-provided Broadcom card with an AC 8260. I can update the
> > > commit log to reflect this.
> > 
> > 
> > Okay, so this makes sense. Those entries are probably formatted for
> > the Broadcom card, which the iwlwifi driver obviously doesn't
> > understand. The best we can do, as I already said, is to ignore values
> > we don't understand.
> 
> 
> This may already be apparent, but Dell sells two versions of the 9350:
> one with the Broadcom adapter and one with the AC 8260.  I just
> happened to find the former version at a deep discount at Costco so
> decided to chance it.  Turns out the Broadcom card is not so good even
> with new kernels so I upgraded.  Anyway, since Paul is seeing the same
> issue I don't think the values are intended to be Broadcom-specific.

Right, this is not for Broadcom.  I found out that this is something
Intel specifies as part of the entire platform's ACPI recommendations.


> On Wed, 2016-10-12 at 17:21 +0300, Luca Coelho wrote:
> > And, the values in the SPLX structs are being changed here, to DOM1,
> > LIM1, TIM1 etc., before being returned. Â This also matches your
> > description that, at runtime, you got something different than the pure
> > dump. Â If you follow these DOM*, LIM*, TIM* symbols, you'll probably
> > end up getting the values you observed at runtime.
> 
> 
> Probably not important, but it seems that there is some additional
> indirection.  The only values I'm seeing associated with those symbols
> are 8 and 16:
> 
>     $ grep -e 'DOM[0-9]' -e 'LIM[0-9]' -e 'TIM[0-9]' dsdt.dsl | grep -v Store
>     DOM1,   8,
>     LIM1,   16,
>     TIM1,   16,
>     DOM2,   8,
>     LIM2,   16,
>     TIM2,   16,
>     DOM3,   8,
>     LIM3,   16,
>     TIM3,   16,

Yeah, there are often many levels of indirection.  These settings can
be also tied to the configuration that is reachable by the user in the
BIOS setup, so you never know.

The easiest way is probably to run the ASL with acpiexec and execute
the method...

> > I'll send you a patch for testing soon.
> 
> 
> I will keep an eye on the list archive, thanks!

I'll ping you from my Intel address and provide you with a link to the
patch, to make it easier for you. ;)

^ permalink raw reply

* Re: [PATCH iproute2] tc: cls_bpf: handle skip_sw and skip_hw flags
From: Simon Horman @ 2016-10-13  9:06 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Jakub Kicinski, netdev
In-Reply-To: <57FE5EC7.3010405@iogearbox.net>

On Wed, Oct 12, 2016 at 06:03:19PM +0200, Daniel Borkmann wrote:
> On 10/12/2016 05:46 PM, Jakub Kicinski wrote:
> >Add support for controling hardware offload using (now standard)
> >skip_sw and skip_hw flags in cls_bpf.
> >
> >Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> 
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Reviewed-by: Simon Horman <simon.horman@netronome.com>

^ permalink raw reply

* Re: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error
From: Arnd Bergmann @ 2016-10-13  9:09 UTC (permalink / raw)
  To: Mintz, Yuval
  Cc: Ariel Elior, everest-linux-l2@qlogic.com, Alexander Duyck,
	Amrani, Ram, netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	David S. Miller
In-Reply-To: <BL2PR07MB23069C99213F4E89A7DC162A8DDC0@BL2PR07MB2306.namprd07.prod.outlook.com>

On Thursday, October 13, 2016 8:50:21 AM CEST Mintz, Yuval wrote:
> >  config INFINIBAND_QEDR
> > -	tristate "QLogic qede RoCE sources [debug]"
> > +	bool "QLogic qede RoCE sources [debug]"
> 
> Given that the qedr submission is going to turn this back into a tristate,
> are you certain this is a good thing [from compilation coverage perspective]?

I haven't seen that submission, I just looked at the current
state in linux-next. If we want this to be a separately loadable
module, that seems fine too, but then we should fix the Makefile
to do that, and add the necessary Kconfig magic to ensure that
INFINIBAND_QED cannot be built-in when INFINIBAND_QEDR=m.

> > -		if (cond)
> > +		if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) && cond)
> >			qed_rdma_dpm_bar(p_hwfn, p_ptt);
> 
> Why not simply fix the qed_roce.h empty implementation?

Mainly for consistency: we have a couple of interfaces that
are called from the qed driver that are implemented in
qed_roce.c. We can either use a 'static inline' helper for
all of them, or use if(IS_ENABLED()) everywhere. Since this
was the only function that had a helper and that helper
was defined incorrectly, I went with the second option.

> > -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> >  	/* Roce CNQ each requires: 1 status block + 1 CNQ. We divide the
> >  	 * status blocks equally between L2 / RoCE but with consideration as
> >  	 * to how many l2 queues / cnqs we have
> >  	 */
> > -	if (p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
> > +	if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) &&
> > +	    p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
> >  		num_features++;
> > 
> >  		feat_num[QED_RDMA_CNQ] =
> >  			min_t(u32, RESC_NUM(p_hwfn, QED_SB) /
> > num_features,
> >  			      RESC_NUM(p_hwfn, QED_RDMA_CNQ_RAM));
> >  	}
> > -#endif
> 
> Is there any non-cosmetic gain here?
> I would gain that having the comment under the #ifdef is more meaningful
> than having the check in the actual condition.

No, it's purely cosmetic. Moving the comment inside of the if()
block seems fine, I just didn't want to touch that as it was
unrelated.

> > -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> > +	if (!IS_ENABLED(CONFIG_INFINIBAND_QEDR))
> > +		return 0;
> > +
> >  	num_l2_queues = 0;
> >  	for_each_hwfn(cdev, i)
> >  		num_l2_queues += FEAT_NUM(&cdev->hwfns[i],
> > QED_PF_L2_QUE); @@ -738,7 +736,6 @@ static int
> > qed_slowpath_setup_int(struct qed_dev *cdev,
> >  	DP_VERBOSE(cdev, QED_MSG_RDMA, "roce_msix_cnt=%d
> > roce_msix_base=%d\n",
> >  		   cdev->int_params.rdma_msix_cnt,
> >  		   cdev->int_params.rdma_msix_base);
> > -#endif
> 
> While I don't mind, you could have argued is that we're not
> removing enough, not too much.
> I.e., perhaps the rdma_msix_* fields should also have been
> ifdef-ed instead. [in which case this solution would not have worked]

That would add even more #ifdefs though.

	Arnd

^ permalink raw reply

* RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error
From: Mintz, Yuval @ 2016-10-13  8:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ariel Elior, everest-linux-l2@qlogic.com, Alexander Duyck,
	Amrani, Ram, netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	David S. Miller
In-Reply-To: <20161012103340.978726-1-arnd@arndb.de>

>  config INFINIBAND_QEDR
> -	tristate "QLogic qede RoCE sources [debug]"
> +	bool "QLogic qede RoCE sources [debug]"

Given that the qedr submission is going to turn this back into a tristate,
are you certain this is a good thing [from compilation coverage perspective]?

> -		if (cond)
> +		if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) && cond)
>			qed_rdma_dpm_bar(p_hwfn, p_ptt);

Why not simply fix the qed_roce.h empty implementation?

> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
>  	/* Roce CNQ each requires: 1 status block + 1 CNQ. We divide the
>  	 * status blocks equally between L2 / RoCE but with consideration as
>  	 * to how many l2 queues / cnqs we have
>  	 */
> -	if (p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
> +	if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) &&
> +	    p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
>  		num_features++;
> 
>  		feat_num[QED_RDMA_CNQ] =
>  			min_t(u32, RESC_NUM(p_hwfn, QED_SB) /
> num_features,
>  			      RESC_NUM(p_hwfn, QED_RDMA_CNQ_RAM));
>  	}
> -#endif

Is there any non-cosmetic gain here?
I would gain that having the comment under the #ifdef is more meaningful
than having the check in the actual condition.

> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> +	if (!IS_ENABLED(CONFIG_INFINIBAND_QEDR))
> +		return 0;
> +
>  	num_l2_queues = 0;
>  	for_each_hwfn(cdev, i)
>  		num_l2_queues += FEAT_NUM(&cdev->hwfns[i],
> QED_PF_L2_QUE); @@ -738,7 +736,6 @@ static int
> qed_slowpath_setup_int(struct qed_dev *cdev,
>  	DP_VERBOSE(cdev, QED_MSG_RDMA, "roce_msix_cnt=%d
> roce_msix_base=%d\n",
>  		   cdev->int_params.rdma_msix_cnt,
>  		   cdev->int_params.rdma_msix_base);
> -#endif

While I don't mind, you could have argued is that we're not
removing enough, not too much.
I.e., perhaps the rdma_msix_* fields should also have been
ifdef-ed instead. [in which case this solution would not have worked]

> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> -	params->rdma_pf_params.num_qps = QED_ROCE_QPS;
> -	params->rdma_pf_params.min_dpis = QED_ROCE_DPIS;
> -	/* divide by 3 the MRs to avoid MF ILT overflow */
> -	params->rdma_pf_params.num_mrs = RDMA_MAX_TIDS;
> -	params->rdma_pf_params.gl_pi = QED_ROCE_PROTOCOL_INDEX;
> -#endif
> +	if (IS_ENABLED(CONFIG_INFINIBAND_QEDR)) {
> +		params->rdma_pf_params.num_qps = QED_ROCE_QPS;
> +		params->rdma_pf_params.min_dpis = QED_ROCE_DPIS;
> +		/* divide by 3 the MRs to avoid MF ILT overflow */
> +		params->rdma_pf_params.num_mrs = RDMA_MAX_TIDS;
> +		params->rdma_pf_params.gl_pi =
> QED_ROCE_PROTOCOL_INDEX;
> +	}

Likewise

> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> -	case PROTOCOLID_ROCE:
> -		qed_async_roce_event(p_hwfn, p_eqe);
> -		return 0;
> -#endif
>  	case PROTOCOLID_COMMON:
>  		return qed_sriov_eqe_event(p_hwfn,
>  					   p_eqe->opcode,
>  					   p_eqe->echo, &p_eqe->data);
> +	case PROTOCOLID_ROCE:
> +		if (IS_ENABLED(CONFIG_INFINIBAND_QEDR)) {
> +			qed_async_roce_event(p_hwfn, p_eqe);
> +			return 0;
> +		}
> +		/* fallthrough */

Not sure whether it helps readability; It might give the false
Impression that it's possible to receive an async roce event if
roce is compiled-out, which isn't the case.

^ permalink raw reply

* Possible regression on NSFv3/sunrpc timeouts when using NFS_CS_DISCRTRY
From: Stefano Panella @ 2016-10-13  9:29 UTC (permalink / raw)
  To: linux-nfs@vger.kernel.org, netdev@vger.kernel.org

Hi all,

I think there has been a change in the net/sunrpc code introduced with 

commit 9cbc94fb06f98de0e8d393eaff09c790f4c3ba46
Author: Trond Myklebust <trond.myklebust@primarydata.com>
Date:   Sun Feb 8 15:50:27 2015 -0500

    SUNRPC: Remove TCP socket linger code

    Now that we no longer use the partial shutdown code when closing the
    socket, we no longer need to worry about the TCP linger2 state.

    Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>

which has caused some regressions removing the functionality which was mentioned 
in the email thread I have attached below.

We have exactly the same use case as the one in the email thread. We call
    __set_bit(NFS_CS_DISCRTRY, &clp->cl_flags)
also in NSFv3

which means we have been relying on the lingering_timeout code to disconnect
and be able to reconnect quickly if the client IP suddenly changes.

As we stand, before moving to 4.4. kernel, where lingering_timeout functionality 
has been removed, we were able to talk again with the NFSv3 share in 60 + 15 seconds
(XS_TCP_LINGER_TO = 15 seconds) while now, after the commit above,  
we need 60 + 924 seconds.

This is because we are keeping the global /proc/sys/net/ipv4/tcp_retries2 = 15 as it is by 
default and we would not want to risk to change it to something else.

I have two questions:

1) Can we say removing the lingering_timeout functionality caused a regression?
2) Is there any way this could be fixed reintroducing it or putting a new mechanism in place
so we can talk to the share using a new connection before the previous (with the old IP) times
out with tcp_retries2 after 15 minutes?

Please have a look at the thread I have included below and let me know what do you think
about this problem.

If you can recommend a way you would like this to be addressed, I would be happy to 
contribute a patch.

Thanks everyone,

Stefano


THREAD I AM REFERRING TO:
----------------------------------------------------------------------------------------------------------------------

On Wed, 24 Sep 2014 16:39:55 +0100 Benjamin ESTRABAUD <be@xxxxxxxxxx> wrote:

> Hi!
> 
> I've got a scenario where I'm connected to a NFS share on a client, have 
> a file descriptor open as read only (could also be write) on a file from 
> that share, and I'm suddenly changing the IP address of that client.
> 
> Obviously, the NFS share will hang, so if I now try to read the file 
> descriptor I've got open (here in Python), the "read" call will also hang.
> 
> However, the driver seems to attempt to do something (maybe 
> save/determine whether the existing connection can be saved) and then, 
> after about 20 minutes the driver transparently reconnects to the NFS 
> share (which is what I wanted anyways) and the "read" call instantiated 
> earlier simply finishes (I don't even have to re-open the file again or 
> even call "read" again).
> 
> The dmesg prints I get are as follow:
> 
> [ 4424.500380] nfs: server 10.0.2.17 not responding, still trying <-- 
> changed IP address and started reading the file.
> [ 4451.560467] nfs: server 10.0.2.17 OK <--- The NFS share was 
> reconnected, the "read" call completes successfully.

The difference between these timestamps is 27 seconds, which is a lot less
than the "20 minutes" that you quote.  That seems odd.

If you adjust
   /proc/sys/net/ipv4/tcp_retries2

you can reduce the current timeout.
See Documentation/networking/ip-sysctl.txt for details on the setting.

https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt

It claims the default gives an effective timeout of 924 seconds or about 15
minutes.

I just tried and the timeout was 1047 seconds. This is probably the next
retry after 924 seconds.

If I reduce tcp_retries2 to '3' (well below the recommended minimum) I get
a timeout of 5 seconds.
You can possibly find a suitable number that isn't too small...

Alternately you could use NFSv4.  It will close the connection on a timeout.
In the default config I measure a 78 second timeout, which is probably more
acceptable.  This number would respond to the timeo mount option.
If I set that to 100, I get a 28 second timeout.

The same effect could be provided for NFSv3 by setting:

           __set_bit(NFS_CS_DISCRTRY, &clp->cl_flags);

somewhere appropriate.  I wonder why that isn't being done for v3 already...
Probably some subtle protocol difference.

NeilBrown

 
> I would like to know if there was any way to tune this behaviour, 
> telling the NFS driver to reconnect if a share is unavailable after say 
> 10 seconds.
> 
> I tried the following options without any success:
> 
> retry=0; hard/soft; timeo=3; retrans=1; bg/fg
> 
> I am running on a custom distro (homemade embedded distro, not based on 
> anything in particular) running stock kernel 3.10.18 compiled for i686.
> 
> Would anyone know what I could do to force NFS into reconnecting a 
> seemingly "dead" session sooner?
> 
> Thanks in advance for your help.
> 
> Regards,
> 
> Ben - MPSTOR.

^ permalink raw reply

* RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error
From: Mintz, Yuval @ 2016-10-13  9:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ariel Elior, everest-linux-l2@qlogic.com, Alexander Duyck,
	Amrani, Ram, netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	David S. Miller
In-Reply-To: <6935319.6Adbod0g1H@wuerfel>


> > > -		if (cond)
> > > +		if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) && cond)
> > >			qed_rdma_dpm_bar(p_hwfn, p_ptt);
> >
> > Why not simply fix the qed_roce.h empty implementation?
> 
> Mainly for consistency: we have a couple of interfaces that are called from the
> qed driver that are implemented in qed_roce.c. We can either use a 'static inline'
> helper for all of them, or use if(IS_ENABLED()) everywhere. Since this was the
> only function that had a helper and that helper was defined incorrectly, I went
> with the second option.

Actually, that's not the case. I think with this exception, all the rest of the prototypes
in qed_roce.h aren't really needed, as those functions should only be accessed via
the qed_rdma_ops. I'll remove those later [or we can remove them as part of v2].

The genereal qed* preference is to have empty static-inline implementations
in case content is compiled-out [Look at iov for example].

> > > -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> > > +	if (!IS_ENABLED(CONFIG_INFINIBAND_QEDR))
> > > +		return 0;
> > > +
> > >  	num_l2_queues = 0;
> > >  	for_each_hwfn(cdev, i)
> > >  		num_l2_queues += FEAT_NUM(&cdev->hwfns[i],
> QED_PF_L2_QUE); @@
> > > -738,7 +736,6 @@ static int qed_slowpath_setup_int(struct qed_dev
> > > *cdev,
> > >  	DP_VERBOSE(cdev, QED_MSG_RDMA, "roce_msix_cnt=%d
> > > roce_msix_base=%d\n",
> > >  		   cdev->int_params.rdma_msix_cnt,
> > >  		   cdev->int_params.rdma_msix_base); -#endif
> >
> > While I don't mind, you could have argued is that we're not removing
> > enough, not too much.
> > I.e., perhaps the rdma_msix_* fields should also have been ifdef-ed
> > instead. [in which case this solution would not have worked]
> 
> That would add even more #ifdefs though.

I agree. Although I'm never clear on the guidelines for the tradeoff - 
How much memory/code is considered too much so that you'd have
To ifdef code out instead of 'wasting'?
[I obviously don't claim 64 bytes of memory hit that threshold]
 
BTW, are you interested in doing a v2 for this? Or would you prefer
if we'd pick it up from here?

Thanks,
Yuval 

^ permalink raw reply

* Re: [patch net-next RFC 0/6] Add support for offloading packet-sampling
From: Roopa Prabhu @ 2016-10-13  7:29 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
	geert+renesas, stephen, xiyou.wangcong, linux
In-Reply-To: <1476276069-5315-1-git-send-email-jiri@resnulli.us>

On 10/12/16, 5:41 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Add the sample tc action, which allows to sample packet matching
> a classifier. The sample action peeks randomly packets, duplicates them,
> truncates them and adds informative metadata on the packet, for example,
> the input interface and the original packet length. The sampled packets
> are marked to allow matching them and redirecting them to a specific
> collector device.
>
> The sampled packets metadata is packed using ife encapsulation. To do
> that, this patch-set extracts ife logics from the tc_ife action into an
> independent ife module, and uses that functionality to pack the metadata.
> To include all the needed metadata, this patch-set introduces some new
> IFE_META tlv types.
>
> In addition, Add the support for offloading the matchall-sample tc command
> in the Mellanox mlxsw driver, for ingress qdiscs.
>
> Yotam Gigi (6):
>   Introduce ife encapsulation module
>   act_ife: Change to use ife module
>   ife: Introduce new metadata tlv types
>   Introduce sample tc action
>   mlxsw: reg: add the Monitoring Packet Sampling Configuration Register
>   mlxsw: packet sample: Add packet sample offloading support
>

we spoke with yotam about this at netdev1.2. and also remember speaking about this on our switchdev calls:
Today our driver uses NFLOG to log packets to a netlink socket and hsflowd supported by the sflow
people (at http://www.sflow.net/) is capable of reading from a nflog socket. NFLOG has the required netlink
attribute markers for packet header/data (which we can possibly extend). We could also add nflog like action
in tc if needed.

sflow agents like hsflowd are capable of sending packets to an external collector with the required sflow header.
Instead of re-inventing a new API for sflow, would be better to standardize/unify on existing mechanisms.

Also, this patch series requires a new device to be created which can be avoided if we used
existing mechanisms like NFLOG.

^ permalink raw reply


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