Netdev List
 help / color / mirror / Atom feed
* RE: [BUG] bnx2x : lockdep assertion
From: Eric Dumazet @ 2014-01-06 16:49 UTC (permalink / raw)
  To: Ariel Elior
  Cc: Yuval Mintz, davem@davemloft.net, netdev@vger.kernel.org,
	Michal Kalderon
In-Reply-To: <6AE768456CEC4B4A9B2248CB6B87EB3E1BFBE6A6@SJEXCHMB05.corp.ad.broadcom.com>

On Mon, 2014-01-06 at 16:35 +0000, Ariel Elior wrote:

> Hi Eric,
> This was introduced by the low latency patch which added a 'might_sleep()' to napi_disable a while ago.
> There is a fix in the works.
> Thanks,

Hmm, commit 80c33ddd31d0e801953 is 3 months old.

Do you have an idea of ETA for the fix ?

Thanks !

^ permalink raw reply

* Re: [net-next 08/15] i40e: acknowledge VFLR when disabling SR-IOV
From: Sergei Shtylyov @ 2014-01-06 16:43 UTC (permalink / raw)
  To: Jeff Kirsher, Mitch Williams
  Cc: davem, netdev, gospo, sassmann, Jesse Brandeburg
In-Reply-To: <1389011436-18424-9-git-send-email-jeffrey.t.kirsher@intel.com>

On 06.01.2014 16:30, Jeff Kirsher wrote:

> From: Mitch Williams <mitch.a.williams@intel.com>

> When SR-IOV is disabled, the (now nonexistent) virtual function
> devices undergo a VFLR event. We don't need to handle this event
> because the VFs are gone, but we do need to tell the HW that they are
> complete. This fixes an issue with a phantom VFLR and broken VFs when
> SR-IOV is re-enabled.

> Change-Id: I7580b49ded0158172a85b14661ec212af77000c8
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Sibai Li <sibai.li@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index f92404c..e91f9d7 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
[...]
> @@ -748,8 +750,17 @@ void i40e_free_vfs(struct i40e_pf *pf)
>   	kfree(pf->vf);
>   	pf->vf = NULL;
>
> -	if (!i40e_vfs_are_assigned(pf))
> +	if (!i40e_vfs_are_assigned(pf)) {
>   		pci_disable_sriov(pf->pdev);
> +		/* Acknowledge VFLR for all VFS. Without this, VFs will fail to
> +		 * work correctly when SR-IOV gets re-enabled.
> +		 */
> +		for (vf_id = 0; vf_id < tmp; vf_id++) {
> +			reg_idx = (hw->func_caps.vf_base_id + vf_id) / 32;
> +			bit_idx = (hw->func_caps.vf_base_id + vf_id) % 32;
> +			wr32(hw, I40E_GLGEN_VFLRSTAT(reg_idx), (1 << bit_idx));
> +		}
> +	}
>   	else

    } and *else* should be on the same line. And the *else* arm should also 
have {} now.

>   		dev_warn(&pf->pdev->dev,
>   			 "unable to disable SR-IOV because VFs are assigned.\n");

WBR, Sergei

^ permalink raw reply

* Re: [PATCH V1 net-next 2/2] net: Add UDP GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-06 16:38 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Or Gerlitz, Jerry Chu, Eric Dumazet, Herbert Xu,
	Linux Netdev List, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <CAJZOPZJ2DRP13LRzcu__rh=JxY7oAvCQ37dCpmO40ptiRn5hFw@mail.gmail.com>

On Mon, Jan 6, 2014 at 3:32 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
> As the GUE RFC states, with UDP encapsulation, the encap protocol is
> actually derived from the UDP destination port. With that assumption
> at hand, we can have the udp gro code to export callbacks for upper
> uncapsulation protocol to install a gro handler which has the
> following trivial strucure:
>
> key = <PORT>  data = <gro receive/complete callbacks>

> which plugs nicely into the general framework of the gro stack. The
> udp gro receive code would act as follows
> -- if there's no gro handler set for the packet udp dest port, flush
> - do the normal loop to find match on the packet dest/src udp ports
> - call the protocol (e.g VXLAN) gro receive callback
> This would avoid dealing with sockets etc and be very efficient, makes sense?

Or in short -- the destination udp port takes a similar role to the
ethertype for gro l3 handlers or the ip protocol for gro l4 handlers
and we enable encapsulating protocols to plug into the existing
offload_callbacks framework, simple.

^ permalink raw reply

* Re: [PATCH 03/12] netfilter: nfnetlink_queue: enable UID/GID socket info retrieval
From: Pablo Neira Ayuso @ 2014-01-06 16:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Valentina Giusti, netfilter-devel, davem, netdev
In-Reply-To: <1389022353.12212.176.camel@edumazet-glaptop2.roam.corp.google.com>

On Mon, Jan 06, 2014 at 07:32:33AM -0800, Eric Dumazet wrote:
> On Mon, 2014-01-06 at 00:12 +0100, Pablo Neira Ayuso wrote:
> > From: Valentina Giusti <valentina.giusti@bmw-carit.de>
> > 
> > Thanks to commits 41063e9 (ipv4: Early TCP socket demux) and 421b388
> > (udp: ipv4: Add udp early demux) it is now possible to parse UID and
> > GID socket info also for incoming TCP and UDP connections. Having
> > this info available, it is convenient to let NFQUEUE parse it in
> > order to improve and refine the traffic analysis in userspace.
> > 
> > Signed-off-by: Valentina Giusti <valentina.giusti@bmw-carit.de>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
[...]
> net/netfilter/nfnetlink_queue_core.c: In function 'nfqnl_put_sk_uidgid':
> net/netfilter/nfnetlink_queue_core.c:304:35: error: 'TCP_TIME_WAIT' undeclared (first use in this function)
> net/netfilter/nfnetlink_queue_core.c:304:35: note: each undeclared identifier is reported only once for each function it appears in
> make[3]: *** [net/netfilter/nfnetlink_queue_core.o] Error 1
> 
> Fix is obvious (#include <net/tcp_states.h>), but I have to run...

Just sent this patch to David, thanks for reporting.

http://patchwork.ozlabs.org/patch/307367/

^ permalink raw reply

* [PATCH v3 5/5] alx: add stats to ethtool
From: Sabrina Dubroca @ 2014-01-06 16:33 UTC (permalink / raw)
  To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389026023-18743-1-git-send-email-sd@queasysnail.net>

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/ethernet/atheros/alx/ethtool.c | 101 +++++++++++++++++++++++++++++
 drivers/net/ethernet/atheros/alx/hw.h      |   7 +-
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c
index 45b3650..08e22df 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -46,6 +46,66 @@
 #include "reg.h"
 #include "hw.h"
 
+/* The order of these strings must match the order of the fields in
+ * struct alx_hw_stats
+ * See hw.h
+ */
+static const char alx_gstrings_stats[][ETH_GSTRING_LEN] = {
+	"rx_packets",
+	"rx_bcast_packets",
+	"rx_mcast_packets",
+	"rx_pause_packets",
+	"rx_ctrl_packets",
+	"rx_fcs_errors",
+	"rx_length_errors",
+	"rx_bytes",
+	"rx_runt_packets",
+	"rx_fragments",
+	"rx_64B_or_less_packets",
+	"rx_65B_to_127B_packets",
+	"rx_128B_to_255B_packets",
+	"rx_256B_to_511B_packets",
+	"rx_512B_to_1023B_packets",
+	"rx_1024B_to_1518B_packets",
+	"rx_1519B_to_mtu_packets",
+	"rx_oversize_packets",
+	"rx_rxf_ov_drop_packets",
+	"rx_rrd_ov_drop_packets",
+	"rx_align_errors",
+	"rx_bcast_bytes",
+	"rx_mcast_bytes",
+	"rx_address_errors",
+	"tx_packets",
+	"tx_bcast_packets",
+	"tx_mcast_packets",
+	"tx_pause_packets",
+	"tx_exc_defer_packets",
+	"tx_ctrl_packets",
+	"tx_defer_packets",
+	"tx_bytes",
+	"tx_64B_or_less_packets",
+	"tx_65B_to_127B_packets",
+	"tx_128B_to_255B_packets",
+	"tx_256B_to_511B_packets",
+	"tx_512B_to_1023B_packets",
+	"tx_1024B_to_1518B_packets",
+	"tx_1519B_to_mtu_packets",
+	"tx_single_collision",
+	"tx_multiple_collisions",
+	"tx_late_collision",
+	"tx_abort_collision",
+	"tx_underrun",
+	"tx_trd_eop",
+	"tx_length_errors",
+	"tx_trunc_packets",
+	"tx_bcast_bytes",
+	"tx_mcast_bytes",
+	"tx_update",
+};
+
+#define ALX_NUM_STATS ARRAY_SIZE(alx_gstrings_stats)
+
+
 static u32 alx_get_supported_speeds(struct alx_hw *hw)
 {
 	u32 supported = SUPPORTED_10baseT_Half |
@@ -201,6 +261,44 @@ static void alx_set_msglevel(struct net_device *netdev, u32 data)
 	alx->msg_enable = data;
 }
 
+static void alx_get_ethtool_stats(struct net_device *netdev,
+				  struct ethtool_stats *estats, u64 *data)
+{
+	struct alx_priv *alx = netdev_priv(netdev);
+	struct alx_hw *hw = &alx->hw;
+
+	spin_lock(&alx->stats_lock);
+
+	alx_update_hw_stats(hw);
+	BUILD_BUG_ON(sizeof(hw->stats) - offsetof(struct alx_hw_stats, rx_ok) <
+		     ALX_NUM_STATS * sizeof(u64));
+	memcpy(data, &hw->stats.rx_ok, ALX_NUM_STATS * sizeof(u64));
+
+	spin_unlock(&alx->stats_lock);
+}
+
+static void alx_get_strings(struct net_device *netdev, u32 stringset, u8 *buf)
+{
+	switch (stringset) {
+	case ETH_SS_STATS:
+		memcpy(buf, &alx_gstrings_stats, sizeof(alx_gstrings_stats));
+		break;
+	default:
+		WARN_ON(1);
+		break;
+	}
+}
+
+static int alx_get_sset_count(struct net_device *netdev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_STATS:
+		return ALX_NUM_STATS;
+	default:
+		return -EINVAL;
+	}
+}
+
 const struct ethtool_ops alx_ethtool_ops = {
 	.get_settings	= alx_get_settings,
 	.set_settings	= alx_set_settings,
@@ -209,4 +307,7 @@ const struct ethtool_ops alx_ethtool_ops = {
 	.get_msglevel	= alx_get_msglevel,
 	.set_msglevel	= alx_set_msglevel,
 	.get_link	= ethtool_op_get_link,
+	.get_strings	= alx_get_strings,
+	.get_sset_count	= alx_get_sset_count,
+	.get_ethtool_stats	= alx_get_ethtool_stats,
 };
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index a97edfb..5978253 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -381,7 +381,12 @@ struct alx_rrd {
 				 ALX_ISR_RX_Q6 | \
 				 ALX_ISR_RX_Q7)
 
-/* Statistics counters collected by the MAC */
+/* Statistics counters collected by the MAC
+ *
+ * The order of the fields must match the strings in alx_gstrings_stats
+ * All stats fields should be u64
+ * See ethtool.c
+ */
 struct alx_hw_stats {
 	/* rx */
 	u64 rx_ok;
-- 
1.8.5.2

^ permalink raw reply related

* [PATCH v3 4/5] alx: add alx_get_stats64 operation
From: Sabrina Dubroca @ 2014-01-06 16:33 UTC (permalink / raw)
  To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389026023-18743-1-git-send-email-sd@queasysnail.net>

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/ethernet/atheros/alx/alx.h  |  3 +++
 drivers/net/ethernet/atheros/alx/main.c | 44 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/atheros/alx/alx.h b/drivers/net/ethernet/atheros/alx/alx.h
index d71103d..8fc93c5 100644
--- a/drivers/net/ethernet/atheros/alx/alx.h
+++ b/drivers/net/ethernet/atheros/alx/alx.h
@@ -106,6 +106,9 @@ struct alx_priv {
 	u16 msg_enable;
 
 	bool msi;
+
+	/* protects hw.stats */
+	spinlock_t stats_lock;
 };
 
 extern const struct ethtool_ops alx_ethtool_ops;
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index c3c4c26..912d732 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1166,10 +1166,54 @@ static void alx_poll_controller(struct net_device *netdev)
 }
 #endif
 
+static struct rtnl_link_stats64 *alx_get_stats64(struct net_device *dev,
+					struct rtnl_link_stats64 *net_stats)
+{
+	struct alx_priv *alx = netdev_priv(dev);
+	struct alx_hw_stats *hw_stats = &alx->hw.stats;
+
+	spin_lock(&alx->stats_lock);
+
+	alx_update_hw_stats(&alx->hw);
+
+	net_stats->tx_packets = hw_stats->tx_ok;
+	net_stats->tx_bytes   = hw_stats->tx_byte_cnt;
+	net_stats->rx_packets = hw_stats->rx_ok;
+	net_stats->rx_bytes   = hw_stats->rx_byte_cnt;
+	net_stats->multicast  = hw_stats->rx_mcast;
+	net_stats->collisions = hw_stats->tx_single_col +
+				hw_stats->tx_multi_col * 2 +
+				hw_stats->tx_late_col + hw_stats->tx_abort_col;
+
+	net_stats->rx_errors  = hw_stats->rx_frag + hw_stats->rx_fcs_err +
+				hw_stats->rx_len_err + hw_stats->rx_ov_sz +
+				hw_stats->rx_ov_rrd + hw_stats->rx_align_err;
+
+	net_stats->rx_fifo_errors   = hw_stats->rx_ov_rxf;
+	net_stats->rx_length_errors = hw_stats->rx_len_err;
+	net_stats->rx_crc_errors    = hw_stats->rx_fcs_err;
+	net_stats->rx_frame_errors  = hw_stats->rx_align_err;
+	net_stats->rx_over_errors   = hw_stats->rx_ov_rrd + hw_stats->rx_ov_rxf;
+
+	net_stats->rx_missed_errors = hw_stats->rx_ov_rrd + hw_stats->rx_ov_rxf;
+
+	net_stats->tx_errors = hw_stats->tx_late_col + hw_stats->tx_abort_col +
+			       hw_stats->tx_underrun + hw_stats->tx_trunc;
+
+	net_stats->tx_aborted_errors = hw_stats->tx_abort_col;
+	net_stats->tx_fifo_errors    = hw_stats->tx_underrun;
+	net_stats->tx_window_errors  = hw_stats->tx_late_col;
+
+	spin_unlock(&alx->stats_lock);
+
+	return net_stats;
+}
+
 static const struct net_device_ops alx_netdev_ops = {
 	.ndo_open               = alx_open,
 	.ndo_stop               = alx_stop,
 	.ndo_start_xmit         = alx_start_xmit,
+	.ndo_get_stats64        = alx_get_stats64,
 	.ndo_set_rx_mode        = alx_set_rx_mode,
 	.ndo_validate_addr      = eth_validate_addr,
 	.ndo_set_mac_address    = alx_set_mac_address,
-- 
1.8.5.2

^ permalink raw reply related

* [PATCH v3 3/5] alx: add stats update function
From: Sabrina Dubroca @ 2014-01-06 16:33 UTC (permalink / raw)
  To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389026023-18743-1-git-send-email-sd@queasysnail.net>

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/ethernet/atheros/alx/hw.c | 58 +++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/atheros/alx/hw.h |  4 +++
 2 files changed, 62 insertions(+)

diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index 1e8c24a..7712f06 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -1050,3 +1050,61 @@ bool alx_get_phy_info(struct alx_hw *hw)
 
 	return true;
 }
+
+void alx_update_hw_stats(struct alx_hw *hw)
+{
+	/* RX stats */
+	hw->stats.rx_ok          += alx_read_mem32(hw, ALX_MIB_RX_OK);
+	hw->stats.rx_bcast       += alx_read_mem32(hw, ALX_MIB_RX_BCAST);
+	hw->stats.rx_mcast       += alx_read_mem32(hw, ALX_MIB_RX_MCAST);
+	hw->stats.rx_pause       += alx_read_mem32(hw, ALX_MIB_RX_PAUSE);
+	hw->stats.rx_ctrl        += alx_read_mem32(hw, ALX_MIB_RX_CTRL);
+	hw->stats.rx_fcs_err     += alx_read_mem32(hw, ALX_MIB_RX_FCS_ERR);
+	hw->stats.rx_len_err     += alx_read_mem32(hw, ALX_MIB_RX_LEN_ERR);
+	hw->stats.rx_byte_cnt    += alx_read_mem32(hw, ALX_MIB_RX_BYTE_CNT);
+	hw->stats.rx_runt        += alx_read_mem32(hw, ALX_MIB_RX_RUNT);
+	hw->stats.rx_frag        += alx_read_mem32(hw, ALX_MIB_RX_FRAG);
+	hw->stats.rx_sz_64B      += alx_read_mem32(hw, ALX_MIB_RX_SZ_64B);
+	hw->stats.rx_sz_127B     += alx_read_mem32(hw, ALX_MIB_RX_SZ_127B);
+	hw->stats.rx_sz_255B     += alx_read_mem32(hw, ALX_MIB_RX_SZ_255B);
+	hw->stats.rx_sz_511B     += alx_read_mem32(hw, ALX_MIB_RX_SZ_511B);
+	hw->stats.rx_sz_1023B    += alx_read_mem32(hw, ALX_MIB_RX_SZ_1023B);
+	hw->stats.rx_sz_1518B    += alx_read_mem32(hw, ALX_MIB_RX_SZ_1518B);
+	hw->stats.rx_sz_max      += alx_read_mem32(hw, ALX_MIB_RX_SZ_MAX);
+	hw->stats.rx_ov_sz       += alx_read_mem32(hw, ALX_MIB_RX_OV_SZ);
+	hw->stats.rx_ov_rxf      += alx_read_mem32(hw, ALX_MIB_RX_OV_RXF);
+	hw->stats.rx_ov_rrd      += alx_read_mem32(hw, ALX_MIB_RX_OV_RRD);
+	hw->stats.rx_align_err   += alx_read_mem32(hw, ALX_MIB_RX_ALIGN_ERR);
+	hw->stats.rx_bc_byte_cnt += alx_read_mem32(hw, ALX_MIB_RX_BCCNT);
+	hw->stats.rx_mc_byte_cnt += alx_read_mem32(hw, ALX_MIB_RX_MCCNT);
+	hw->stats.rx_err_addr    += alx_read_mem32(hw, ALX_MIB_RX_ERRADDR);
+
+	/* TX stats */
+	hw->stats.tx_ok          += alx_read_mem32(hw, ALX_MIB_TX_OK);
+	hw->stats.tx_bcast       += alx_read_mem32(hw, ALX_MIB_TX_BCAST);
+	hw->stats.tx_mcast       += alx_read_mem32(hw, ALX_MIB_TX_MCAST);
+	hw->stats.tx_pause       += alx_read_mem32(hw, ALX_MIB_TX_PAUSE);
+	hw->stats.tx_exc_defer   += alx_read_mem32(hw, ALX_MIB_TX_EXC_DEFER);
+	hw->stats.tx_ctrl        += alx_read_mem32(hw, ALX_MIB_TX_CTRL);
+	hw->stats.tx_defer       += alx_read_mem32(hw, ALX_MIB_TX_DEFER);
+	hw->stats.tx_byte_cnt    += alx_read_mem32(hw, ALX_MIB_TX_BYTE_CNT);
+	hw->stats.tx_sz_64B      += alx_read_mem32(hw, ALX_MIB_TX_SZ_64B);
+	hw->stats.tx_sz_127B     += alx_read_mem32(hw, ALX_MIB_TX_SZ_127B);
+	hw->stats.tx_sz_255B     += alx_read_mem32(hw, ALX_MIB_TX_SZ_255B);
+	hw->stats.tx_sz_511B     += alx_read_mem32(hw, ALX_MIB_TX_SZ_511B);
+	hw->stats.tx_sz_1023B    += alx_read_mem32(hw, ALX_MIB_TX_SZ_1023B);
+	hw->stats.tx_sz_1518B    += alx_read_mem32(hw, ALX_MIB_TX_SZ_1518B);
+	hw->stats.tx_sz_max      += alx_read_mem32(hw, ALX_MIB_TX_SZ_MAX);
+	hw->stats.tx_single_col  += alx_read_mem32(hw, ALX_MIB_TX_SINGLE_COL);
+	hw->stats.tx_multi_col   += alx_read_mem32(hw, ALX_MIB_TX_MULTI_COL);
+	hw->stats.tx_late_col    += alx_read_mem32(hw, ALX_MIB_TX_LATE_COL);
+	hw->stats.tx_abort_col   += alx_read_mem32(hw, ALX_MIB_TX_ABORT_COL);
+	hw->stats.tx_underrun    += alx_read_mem32(hw, ALX_MIB_TX_UNDERRUN);
+	hw->stats.tx_trd_eop     += alx_read_mem32(hw, ALX_MIB_TX_TRD_EOP);
+	hw->stats.tx_len_err     += alx_read_mem32(hw, ALX_MIB_TX_LEN_ERR);
+	hw->stats.tx_trunc       += alx_read_mem32(hw, ALX_MIB_TX_TRUNC);
+	hw->stats.tx_bc_byte_cnt += alx_read_mem32(hw, ALX_MIB_TX_BCCNT);
+	hw->stats.tx_mc_byte_cnt += alx_read_mem32(hw, ALX_MIB_TX_MCCNT);
+
+	hw->stats.update         += alx_read_mem32(hw, ALX_MIB_UPDATE);
+}
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 1c1a0ee..a97edfb 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -482,6 +482,9 @@ struct alx_hw {
 
 	/* PHY link patch flag */
 	bool lnk_patch;
+
+	/* cumulated stats from the hardware (registers are cleared on read) */
+	struct alx_hw_stats stats;
 };
 
 static inline int alx_hw_revision(struct alx_hw *hw)
@@ -549,6 +552,7 @@ bool alx_phy_configured(struct alx_hw *hw);
 void alx_configure_basic(struct alx_hw *hw);
 void alx_disable_rss(struct alx_hw *hw);
 bool alx_get_phy_info(struct alx_hw *hw);
+void alx_update_hw_stats(struct alx_hw *hw);
 
 static inline u32 alx_speed_to_ethadv(int speed, u8 duplex)
 {
-- 
1.8.5.2

^ permalink raw reply related

* RE: [BUG] bnx2x : lockdep assertion
From: Ariel Elior @ 2014-01-06 16:35 UTC (permalink / raw)
  To: Eric Dumazet, Yuval Mintz
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Michal Kalderon
In-Reply-To: <1389025680.12212.178.camel@edumazet-glaptop2.roam.corp.google.com>

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Monday, January 06, 2014 6:28 PM
> To: Yuval Mintz
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Ariel Elior; Michal
> Kalderon
> Subject: [BUG] bnx2x : lockdep assertion
> 
> Caught this with latest net-next tree :
> 
> Does this ring a bell to you guys ?
> 

Hi Eric,
This was introduced by the low latency patch which added a 'might_sleep()' to napi_disable a while ago.
There is a fix in the works.
Thanks,
Ariel 


^ permalink raw reply

* [PATCH v3 2/5] alx: add constants for the stats fields
From: Sabrina Dubroca @ 2014-01-06 16:33 UTC (permalink / raw)
  To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389026023-18743-1-git-send-email-sd@queasysnail.net>

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/ethernet/atheros/alx/reg.h | 52 +++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/reg.h b/drivers/net/ethernet/atheros/alx/reg.h
index e4358c9..af006b4 100644
--- a/drivers/net/ethernet/atheros/alx/reg.h
+++ b/drivers/net/ethernet/atheros/alx/reg.h
@@ -404,15 +404,59 @@
 
 /* MIB */
 #define ALX_MIB_BASE					0x1700
+
 #define ALX_MIB_RX_OK					(ALX_MIB_BASE + 0)
+#define ALX_MIB_RX_BCAST				(ALX_MIB_BASE + 4)
+#define ALX_MIB_RX_MCAST				(ALX_MIB_BASE + 8)
+#define ALX_MIB_RX_PAUSE				(ALX_MIB_BASE + 12)
+#define ALX_MIB_RX_CTRL					(ALX_MIB_BASE + 16)
+#define ALX_MIB_RX_FCS_ERR				(ALX_MIB_BASE + 20)
+#define ALX_MIB_RX_LEN_ERR				(ALX_MIB_BASE + 24)
+#define ALX_MIB_RX_BYTE_CNT				(ALX_MIB_BASE + 28)
+#define ALX_MIB_RX_RUNT					(ALX_MIB_BASE + 32)
+#define ALX_MIB_RX_FRAG					(ALX_MIB_BASE + 36)
+#define ALX_MIB_RX_SZ_64B				(ALX_MIB_BASE + 40)
+#define ALX_MIB_RX_SZ_127B				(ALX_MIB_BASE + 44)
+#define ALX_MIB_RX_SZ_255B				(ALX_MIB_BASE + 48)
+#define ALX_MIB_RX_SZ_511B				(ALX_MIB_BASE + 52)
+#define ALX_MIB_RX_SZ_1023B				(ALX_MIB_BASE + 56)
+#define ALX_MIB_RX_SZ_1518B				(ALX_MIB_BASE + 60)
+#define ALX_MIB_RX_SZ_MAX				(ALX_MIB_BASE + 64)
+#define ALX_MIB_RX_OV_SZ				(ALX_MIB_BASE + 68)
+#define ALX_MIB_RX_OV_RXF				(ALX_MIB_BASE + 72)
+#define ALX_MIB_RX_OV_RRD				(ALX_MIB_BASE + 76)
+#define ALX_MIB_RX_ALIGN_ERR				(ALX_MIB_BASE + 80)
+#define ALX_MIB_RX_BCCNT				(ALX_MIB_BASE + 84)
+#define ALX_MIB_RX_MCCNT				(ALX_MIB_BASE + 88)
 #define ALX_MIB_RX_ERRADDR				(ALX_MIB_BASE + 92)
+
 #define ALX_MIB_TX_OK					(ALX_MIB_BASE + 96)
+#define ALX_MIB_TX_BCAST				(ALX_MIB_BASE + 100)
+#define ALX_MIB_TX_MCAST				(ALX_MIB_BASE + 104)
+#define ALX_MIB_TX_PAUSE				(ALX_MIB_BASE + 108)
+#define ALX_MIB_TX_EXC_DEFER				(ALX_MIB_BASE + 112)
+#define ALX_MIB_TX_CTRL					(ALX_MIB_BASE + 116)
+#define ALX_MIB_TX_DEFER				(ALX_MIB_BASE + 120)
+#define ALX_MIB_TX_BYTE_CNT				(ALX_MIB_BASE + 124)
+#define ALX_MIB_TX_SZ_64B				(ALX_MIB_BASE + 128)
+#define ALX_MIB_TX_SZ_127B				(ALX_MIB_BASE + 132)
+#define ALX_MIB_TX_SZ_255B				(ALX_MIB_BASE + 136)
+#define ALX_MIB_TX_SZ_511B				(ALX_MIB_BASE + 140)
+#define ALX_MIB_TX_SZ_1023B				(ALX_MIB_BASE + 144)
+#define ALX_MIB_TX_SZ_1518B				(ALX_MIB_BASE + 148)
+#define ALX_MIB_TX_SZ_MAX				(ALX_MIB_BASE + 152)
+#define ALX_MIB_TX_SINGLE_COL				(ALX_MIB_BASE + 156)
+#define ALX_MIB_TX_MULTI_COL				(ALX_MIB_BASE + 160)
+#define ALX_MIB_TX_LATE_COL				(ALX_MIB_BASE + 164)
+#define ALX_MIB_TX_ABORT_COL				(ALX_MIB_BASE + 168)
+#define ALX_MIB_TX_UNDERRUN				(ALX_MIB_BASE + 172)
+#define ALX_MIB_TX_TRD_EOP				(ALX_MIB_BASE + 176)
+#define ALX_MIB_TX_LEN_ERR				(ALX_MIB_BASE + 180)
+#define ALX_MIB_TX_TRUNC				(ALX_MIB_BASE + 184)
+#define ALX_MIB_TX_BCCNT				(ALX_MIB_BASE + 188)
 #define ALX_MIB_TX_MCCNT				(ALX_MIB_BASE + 192)
+#define ALX_MIB_UPDATE					(ALX_MIB_BASE + 196)
 
-#define ALX_RX_STATS_BIN				ALX_MIB_RX_OK
-#define ALX_RX_STATS_END				ALX_MIB_RX_ERRADDR
-#define ALX_TX_STATS_BIN				ALX_MIB_TX_OK
-#define ALX_TX_STATS_END				ALX_MIB_TX_MCCNT
 
 #define ALX_ISR						0x1600
 #define ALX_ISR_DIS					BIT(31)
-- 
1.8.5.2

^ permalink raw reply related

* [PATCH net-next] netfilter: nfnetlink_queue: fix compilation problem due missing header
From: Pablo Neira Ayuso @ 2014-01-06 16:34 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

In 08c0cad ("netfilter: nfnetlink_queue: enable UID/GID socket info
retrieval"), the header that defines TCP_TIME_WAIT.

net/netfilter/nfnetlink_queue_core.c: In function 'nfqnl_put_sk_uidgid':
net/netfilter/nfnetlink_queue_core.c:304:35: error: 'TCP_TIME_WAIT' undeclared (first use in this function)
net/netfilter/nfnetlink_queue_core.c:304:35: note: each undeclared identifier is reported only once for each function it appears in make[3]: *** [net/netfilter/nfnetlink_queue_core.o] Error 1

Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
Hi David,

Please, put this patch into net-next. It seems this compilation problem is
depending on the .config selections as I didn't reproducible when testing the
patches.

Thanks!

 net/netfilter/nfnetlink_queue_core.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index d3cf12b..ce20bd8 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -32,6 +32,7 @@
 #include <net/netfilter/nf_queue.h>
 #include <net/netns/generic.h>
 #include <net/netfilter/nfnetlink_queue.h>
+#include <net/tcp_states.h>
 
 #include <linux/atomic.h>
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v3 1/5] alx: add a hardware stats structure
From: Sabrina Dubroca @ 2014-01-06 16:33 UTC (permalink / raw)
  To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389026023-18743-1-git-send-email-sd@queasysnail.net>

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/ethernet/atheros/alx/hw.h | 58 +++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 96f3b43..1c1a0ee 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -381,6 +381,64 @@ struct alx_rrd {
 				 ALX_ISR_RX_Q6 | \
 				 ALX_ISR_RX_Q7)
 
+/* Statistics counters collected by the MAC */
+struct alx_hw_stats {
+	/* rx */
+	u64 rx_ok;
+	u64 rx_bcast;
+	u64 rx_mcast;
+	u64 rx_pause;
+	u64 rx_ctrl;
+	u64 rx_fcs_err;
+	u64 rx_len_err;
+	u64 rx_byte_cnt;
+	u64 rx_runt;
+	u64 rx_frag;
+	u64 rx_sz_64B;
+	u64 rx_sz_127B;
+	u64 rx_sz_255B;
+	u64 rx_sz_511B;
+	u64 rx_sz_1023B;
+	u64 rx_sz_1518B;
+	u64 rx_sz_max;
+	u64 rx_ov_sz;
+	u64 rx_ov_rxf;
+	u64 rx_ov_rrd;
+	u64 rx_align_err;
+	u64 rx_bc_byte_cnt;
+	u64 rx_mc_byte_cnt;
+	u64 rx_err_addr;
+
+	/* tx */
+	u64 tx_ok;
+	u64 tx_bcast;
+	u64 tx_mcast;
+	u64 tx_pause;
+	u64 tx_exc_defer;
+	u64 tx_ctrl;
+	u64 tx_defer;
+	u64 tx_byte_cnt;
+	u64 tx_sz_64B;
+	u64 tx_sz_127B;
+	u64 tx_sz_255B;
+	u64 tx_sz_511B;
+	u64 tx_sz_1023B;
+	u64 tx_sz_1518B;
+	u64 tx_sz_max;
+	u64 tx_single_col;
+	u64 tx_multi_col;
+	u64 tx_late_col;
+	u64 tx_abort_col;
+	u64 tx_underrun;
+	u64 tx_trd_eop;
+	u64 tx_len_err;
+	u64 tx_trunc;
+	u64 tx_bc_byte_cnt;
+	u64 tx_mc_byte_cnt;
+	u64 update;
+};
+
+
 /* maximum interrupt vectors for msix */
 #define ALX_MAX_MSIX_INTRS	16
 
-- 
1.8.5.2

^ permalink raw reply related

* [PATCH v2 0/5] alx: add statistics
From: Sabrina Dubroca @ 2014-01-06 16:33 UTC (permalink / raw)
  To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca

Currently, the alx driver doesn't support statistics [1,2]. The
original alx driver [3] that Johannes Berg modified provided
statistics. This patch is an adaptation of the statistics code from
the original driver to the alx driver included in the kernel.


v3:
 - renamed __alx_update_hw_stats to alx_update_hw_stats (Stephen Hemminger)

v2:
 - use u64 instead of unsigned long  (Ben Hutchings)
 - implement ndo_get_stats64 instead of ndo_get_stats (Ben Hutchings)
 - use EINVAL instead of ENOTSUPP  (Ben Hutchings)
 - add BUILD_BUG_ON to check the size of the stats (Johannes Berg, Ben
   Hutchings)
 - add a comment regarding persistence of the stats (Stephen Hemminger)
 - align assignments in __alx_update_hw_stats


[1] https://bugzilla.kernel.org/show_bug.cgi?id=63401
[2] http://www.spinics.net/lists/netdev/msg245544.html
[3] https://github.com/mcgrof/alx


Sabrina Dubroca (5):
  alx: add a hardware stats structure
  alx: add constants for the stats fields
  alx: add stats update function
  alx: add alx_get_stats64 operation
  alx: add stats to ethtool

 drivers/net/ethernet/atheros/alx/alx.h     |   3 +
 drivers/net/ethernet/atheros/alx/ethtool.c | 101 +++++++++++++++++++++++++++++
 drivers/net/ethernet/atheros/alx/hw.c      |  58 +++++++++++++++++
 drivers/net/ethernet/atheros/alx/hw.h      |  67 +++++++++++++++++++
 drivers/net/ethernet/atheros/alx/main.c    |  44 +++++++++++++
 drivers/net/ethernet/atheros/alx/reg.h     |  52 +++++++++++++--
 6 files changed, 321 insertions(+), 4 deletions(-)

-- 
1.8.5.2

^ permalink raw reply

* [BUG] bnx2x : lockdep assertion
From: Eric Dumazet @ 2014-01-06 16:28 UTC (permalink / raw)
  To: Yuval Mintz; +Cc: davem, netdev, ariele, Michal Kalderon
In-Reply-To: <1388939634-19948-3-git-send-email-yuvalmin@broadcom.com>

Caught this with latest net-next tree :

Does this ring a bell to you guys ?


[   27.339221] BUG: sleeping function called from invalid context at include/linux/netdevice.h:486
[   27.347927] in_atomic(): 1, irqs_disabled(): 0, pid: 5672, name: ethtool
[   27.354628] 1 lock held by ethtool/5672:
[   27.354629]  #0:  (rtnl_mutex){+.+.+.}, at: [<ffffffff814f3b97>] rtnl_lock+0x17/0x20
[   27.354641] CPU: 6 PID: 5672 Comm: ethtool Not tainted 3.13.0-dbg-DEV #430
[   27.354644]  0000000000000000 ffff880656881aa8 ffffffff81594454 0000000000000002
[   27.354648]  ffff88065d7f4ad0 ffff880656881ac8 ffffffff810c37af ffff880656881ac8
[   27.354651]  ffff880c50df8000 ffff880656881b18 ffffffffa00b8c18 ffff880c50df0940
[   27.354655] Call Trace:
[   27.354661]  [<ffffffff81594454>] dump_stack+0x4e/0x68
[   27.354666]  [<ffffffff810c37af>] __might_sleep+0xdf/0x110
[   27.354680]  [<ffffffffa00b8c18>] bnx2x_napi_disable.isra.84+0x58/0x130 [bnx2x]
[   27.354689]  [<ffffffffa00bdf65>] bnx2x_netif_stop+0x25/0x40 [bnx2x]
[   27.354696]  [<ffffffffa009123d>] bnx2x_chip_cleanup+0x24d/0x630 [bnx2x]
[   27.354700]  [<ffffffff810a2c55>] ? del_timer_sync+0x5/0xd0
[   27.354708]  [<ffffffffa00c0b40>] bnx2x_nic_unload+0x1f0/0x7f0 [bnx2x]
[   27.354714]  [<ffffffff8118e1af>] ? might_fault+0x5f/0xb0
[   27.354722]  [<ffffffffa00c3a0c>] bnx2x_reload_if_running+0x2c/0x50 [bnx2x]
[   27.354730]  [<ffffffffa00c57e4>] bnx2x_set_ringparam+0x2a4/0x400 [bnx2x]
[   27.354735]  [<ffffffff814ea242>] dev_ethtool+0x752/0x1b30
[   27.354741]  [<ffffffff810e3e1d>] ? trace_hardirqs_on+0xd/0x10
[   27.354743]  [<ffffffff814f3b97>] ? rtnl_lock+0x17/0x20
[   27.354747]  [<ffffffff814fa31d>] dev_ioctl+0x25d/0x5a0
[   27.354752]  [<ffffffff814c9746>] compat_sock_ioctl+0x4e6/0xa50
[   27.354757]  [<ffffffff815a0334>] ? __do_page_fault+0x2c4/0x560
[   27.354763]  [<ffffffff81224c08>] compat_sys_ioctl+0xc8/0x1500
[   27.354765]  [<ffffffff814c832b>] ? sock_map_fd+0x4b/0x70
[   27.354768]  [<ffffffff8159c989>] ? retint_swapgs+0xe/0x13
[   27.354771]  [<ffffffff810e3d45>] ? trace_hardirqs_on_caller+0x105/0x1d0
[   27.354774]  [<ffffffff813207ad>] ? trace_hardirqs_off_thunk+0x3a/0x3c
[   27.354779]  [<ffffffff815a65e5>] sysenter_dispatch+0x7/0x1f
[   27.354781]  [<ffffffff8132076e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[   28.096865] bnx2x 0000:03:00.0 eth0: using MSI-X  IRQs: sp 41  fp[0] 43 ... fp[3] 46
[   28.211869] bnx2x 0000:03:00.0 eth0: NIC Link is Up, 10000 Mbps full duplex, Flow control: ON - receive

^ permalink raw reply

* Re: [PATCH net-next] net: netdev_kobject_init: annotate with __init
From: Daniel Borkmann @ 2014-01-06 16:18 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: davem, netdev
In-Reply-To: <52CAC804.9080603@cogentembedded.com>

On 01/06/2014 04:13 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 06-01-2014 4:20, Daniel Borkmann wrote:
>
>> netdev_kobject_init() is only being called from __init context,
>> that is, net_dev_init(), so annotate it with __init as well, thus
>> the kernel can take this as a hint that the function is used only
>> during the initialization phase and free up used memory resources
>> after its invocation.
>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> [...]
>
>> diff --git a/net/core/net-sysfs.h b/net/core/net-sysfs.h
>> index bd7751e..2745a1b 100644
>> --- a/net/core/net-sysfs.h
>> +++ b/net/core/net-sysfs.h
>> @@ -1,7 +1,7 @@
>>   #ifndef __NET_SYSFS_H__
>>   #define __NET_SYSFS_H__
>>
>> -int netdev_kobject_init(void);
>> +int __init netdev_kobject_init(void);
>
>     There's no need to also annotate function prototype.

Hm, is that general convention? Having this in a header file
annotated (even if not strictly necessary) would probably
better prevent from possible misuse resp. section mismatches
if people forget to look into the actual c file where the
function is being defined.

> WBR, Sergei
>

^ permalink raw reply

* [PATCH] hso: fix handling of modem port SERIAL_STATE notifications
From: Dan Williams @ 2014-01-06 16:07 UTC (permalink / raw)
  To: Dr. H. Nikolaus Schaller
  Cc: Jan Dumon, linux-usb, netdev, LKML, Belisko Marek, e.verdonck
In-Reply-To: <874B65AE-8B15-4427-8EF4-F0CA2BC55062@goldelico.com>

The existing serial state notification handling expected older Option
devices, having a hardcoded assumption that the Modem port was always
USB interface #2.  That isn't true for devices from the past few years.

hso_serial_state_notification is a local cache of a USB Communications
Interface Class SERIAL_STATE notification from the device, and the
USB CDC specification (section 6.3, table 67 "Class-Specific Notifications")
defines wIndex as the USB interface the event applies to.  For hso
devices this will always be the Modem port, as the Modem port is the
only port which is set up to receive them by the driver.

So instead of always expecting USB interface #2, instead validate the
notification with the actual USB interface number of the Modem port.

Signed-off-by: Dan Williams <dcbw@redhat.com>
Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/net/usb/hso.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 86292e6..1a48234 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -181,15 +181,14 @@ enum rx_ctrl_state{
 	RX_SENT,
 	RX_PENDING
 };
 
 #define BM_REQUEST_TYPE (0xa1)
 #define B_NOTIFICATION  (0x20)
 #define W_VALUE         (0x0)
-#define W_INDEX         (0x2)
 #define W_LENGTH        (0x2)
 
 #define B_OVERRUN       (0x1<<6)
 #define B_PARITY        (0x1<<5)
 #define B_FRAMING       (0x1<<4)
 #define B_RING_SIGNAL   (0x1<<3)
 #define B_BREAK         (0x1<<2)
@@ -1483,31 +1482,41 @@ static void tiocmget_intr_callback(struct urb *urb)
 	struct hso_serial *serial = urb->context;
 	struct hso_tiocmget *tiocmget;
 	int status = urb->status;
 	u16 UART_state_bitmap, prev_UART_state_bitmap;
 	struct uart_icount *icount;
 	struct hso_serial_state_notification *serial_state_notification;
 	struct usb_device *usb;
+	int if_num;
 
 	/* Sanity checks */
 	if (!serial)
 		return;
 	if (status) {
 		handle_usb_error(status, __func__, serial->parent);
 		return;
 	}
+
+	/* tiocmget is only supported on HSO_PORT_MODEM */
 	tiocmget = serial->tiocmget;
 	if (!tiocmget)
 		return;
+	BUG_ON((serial->parent->port_spec & HSO_PORT_MASK) != HSO_PORT_MODEM);
+
 	usb = serial->parent->usb;
+	if_num = serial->parent->interface->altsetting->desc.bInterfaceNumber;
+
+	/* wIndex should be the USB interface number of the port to which the
+	 * notification applies, which should always be the Modem port.
+	 */
 	serial_state_notification = &tiocmget->serial_state_notification;
 	if (serial_state_notification->bmRequestType != BM_REQUEST_TYPE ||
 	    serial_state_notification->bNotification != B_NOTIFICATION ||
 	    le16_to_cpu(serial_state_notification->wValue) != W_VALUE ||
-	    le16_to_cpu(serial_state_notification->wIndex) != W_INDEX ||
+	    le16_to_cpu(serial_state_notification->wIndex) != if_num ||
 	    le16_to_cpu(serial_state_notification->wLength) != W_LENGTH) {
 		dev_warn(&usb->dev,
 			 "hso received invalid serial state notification\n");
 		DUMP(serial_state_notification,
 		     sizeof(struct hso_serial_state_notification));
 	} else {
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net] bridge: Fix crash with vlan filtering and tcpdump
From: Vlad Yasevich @ 2014-01-06 16:02 UTC (permalink / raw)
  To: Toshiaki Makita; +Cc: netdev, Toshiaki Makita
In-Reply-To: <1388934147.1906.17.camel@localhost.localdomain>

On 01/05/2014 10:02 AM, Toshiaki Makita wrote:
> On Fri, 2014-01-03 at 11:12 -0500, Vlad Yasevich wrote:
>> When the vlan filtering is enabled on the bridge, but
>> the filter is not configured on the bridge device itself,
>> running tcpdump on the bridge device will result in a
>> an Ooops with NULL pointer dereference.  The reason
>     ^^^^^
> Oops?
> 
>> is that br_pass_frame_up() will bypass the vlan
>> check because promisc flag is set.  It will then try
>> to get the table pointer and process the packet based
>> on the table.  Since the table pointer is NULL, we oops.
>> Catch this special condition in br_handle_vlan().
>>
>> Reported-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> CC: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
> ...
>> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>> index af5ebd1..ea0c29b 100644
>> --- a/net/bridge/br_vlan.c
>> +++ b/net/bridge/br_vlan.c
>> @@ -144,6 +144,18 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
>>  	if (!br->vlan_enabled)
>>  		goto out;
>>  
>> +	/* Vlan filter table must be configrured at this point.  The
>> +	 * only exception is the bridge is set in promisc mode and the
>> +	 * packet is destined for the bridge device.  In this case
>> +	 * pass the packet as is.
>> +	 */
>> +	if (!pv) {
>> +       		if ((br->dev->flags & IFF_PROMISC) && skb->dev == br->dev)
>> +			goto out;
> 
> Currently this function is calling __vlan_put_tag() if port_idx==0.
> You are going to bypass it only if br is promisc mode.
> 
> However it doesn't seem to lead any problem because
> __netif_receive_skb_core() can handle hw accel vlan tag.
> 
> Instead, I doubt if __vlan_put_tag() is necessary in br_handle_vlan()...

I'll need to re-test, but I remember having had to add that code to make
it work on the bridge...

Looking at it again, I agree that special handling for forwarding to the
bridge doesn't seem to be needed.

-vlad

> 
> Thanks,
> Toshiaki Makita
> 
> 

^ permalink raw reply

* Re: [patch iproute2 v2 0/2] add support for IFA_F_MANAGETEMPADDR
From: Hannes Frederic Sowa @ 2014-01-06 16:01 UTC (permalink / raw)
  To: Thomas Haller; +Cc: Jiri Pirko, netdev, stephen, dcbw
In-Reply-To: <1389022909.23397.4.camel@weing>

On Mon, Jan 06, 2014 at 04:41:49PM +0100, Thomas Haller wrote:
> On Sat, 2014-01-04 at 12:35 +0100, Hannes Frederic Sowa wrote:
> > On Sat, Jan 04, 2014 at 12:21:51PM +0100, Thomas Haller wrote:
> > > > > >Also, I saw that NetworkManager switched to install autoconf addresses
> > > > > >as /128, doesn't this break with IFA_F_MANAGETEMPADDR, as you expect a /64
> > > > > >prefixlen?
> > > > > 
> > > > > /64 is required
> > > > 
> > > > Ok, currently NM seems to "violate" that as it installs autoconf addresses
> > > > with 128 prefixlen, so IFA_F_MANAGETEMPADDR should not work on them.
> > > > (currently observed on Fedora 20).
> > > 
> > > True, I noticed that too. I think that is a bug in NM to add the
> > > addresses as /128. Probably, we will fix that soon.
> > 
> > The change could be valid. Otherwise currently NM could not correctly handle
> > prefix information in RAs in some cases:
> > 
> > It is possible to let the client generate an autonomously address in a
> > prefix which is actually not on-link (L=0). Kernel would automatically
> > create prefix route by mistake, if NM tries to install such an address
> > with /64 prefix. This does not happen if the prefix address uses 128
> > prefixlen.
> > 
> > Would be great to have feedback on this, as this could be easily solved by an
> > additional ifa_flag.
> > 
> > Greetings,
> > 
> >   Hannes
> > 
> 
> 
> Hi Hannes,
> 
> 
> good point. I think, the user-space application (NetworkManager) should
> add the autoconf addresses as /64. But the kernel should not generate
> any routes in that case. It's up to the application to add them
> (depending on the on-link flag).
> 
> 
> What do you think about something like:

I would introduce a new flag for that and also make it accessible via
iproute, maybe later. Otherwise IFA_F_MANAGETEMPADDR is overloaded and
doesn't do justice to its name. And since Jiri made new room in ifa_flags,
it shoud be no problem. ;)

Btw. while just reviewing anycast changes, I noticed a funny thing:
Kernel does allocate an anycast address which is the same as the autoconf
address, if NM installs /128 address and you have ipv6 forwarding enabled
(fedora 20 install + ipv6 forwarding because of libvirt). You can see the
addresses pop up in /proc/net/anycast6. I currenlty don't know if that
is problematic.  Everything seems to work here for me. ;)

In the end, I don't think we should install anycast for /128, but I am not
sure, yet.

Greetings,

  Hannes

^ permalink raw reply

* [PATCH] netfilter: nf_conntrack: release conntrack from rcu callback
From: Andrey Vagin @ 2014-01-06 15:54 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netfilter, coreteam, netdev, linux-kernel, vvs, Andrey Vagin,
	Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
	David S. Miller, Cyrill Gorcunov

Lets look at destroy_conntrack:

hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
...
nf_conntrack_free(ct)
	kmem_cache_free(net->ct.nf_conntrack_cachep, ct);

The hash is protected by rcu, so readers look up conntracks without
locks.
A conntrack is removed from the hash, but in this moment a few readers
still can use the conntrack, so if we call kmem_cache_free now, all
readers will read released object.

Bellow you can find more tricky race condition of three tasks.

task 1			task 2			task 3
			nf_conntrack_find_get
			 ____nf_conntrack_find
destroy_conntrack
 hlist_nulls_del_rcu
 nf_conntrack_free
 kmem_cache_free
						__nf_conntrack_alloc
						 kmem_cache_alloc
						 memset(&ct->tuplehash[IP_CT_DIR_MAX],
			 if (nf_ct_is_dying(ct))

In this case the task 2 will not understand, that it uses a wrong
conntrack.

I'm not sure, that I have ever seen this race condition in a real life.
Currently we are investigating a bug, which is reproduced on a few node.
In our case one conntrack is initialized from a few tasks concurrently,
we don't have any other explanation for this.

<2>[46267.083061] kernel BUG at net/ipv4/netfilter/nf_nat_core.c:322!
...
<4>[46267.083951] RIP: 0010:[<ffffffffa01e00a4>]  [<ffffffffa01e00a4>] nf_nat_setup_info+0x564/0x590 [nf_nat]
...
<4>[46267.085549] Call Trace:
<4>[46267.085622]  [<ffffffffa023421b>] alloc_null_binding+0x5b/0xa0 [iptable_nat]
<4>[46267.085697]  [<ffffffffa02342bc>] nf_nat_rule_find+0x5c/0x80 [iptable_nat]
<4>[46267.085770]  [<ffffffffa0234521>] nf_nat_fn+0x111/0x260 [iptable_nat]
<4>[46267.085843]  [<ffffffffa0234798>] nf_nat_out+0x48/0xd0 [iptable_nat]
<4>[46267.085919]  [<ffffffff814841b9>] nf_iterate+0x69/0xb0
<4>[46267.085991]  [<ffffffff81494e70>] ? ip_finish_output+0x0/0x2f0
<4>[46267.086063]  [<ffffffff81484374>] nf_hook_slow+0x74/0x110
<4>[46267.086133]  [<ffffffff81494e70>] ? ip_finish_output+0x0/0x2f0
<4>[46267.086207]  [<ffffffff814b5890>] ? dst_output+0x0/0x20
<4>[46267.086277]  [<ffffffff81495204>] ip_output+0xa4/0xc0
<4>[46267.086346]  [<ffffffff814b65a4>] raw_sendmsg+0x8b4/0x910
<4>[46267.086419]  [<ffffffff814c10fa>] inet_sendmsg+0x4a/0xb0
<4>[46267.086491]  [<ffffffff814459aa>] ? sock_update_classid+0x3a/0x50
<4>[46267.086562]  [<ffffffff81444d67>] sock_sendmsg+0x117/0x140
<4>[46267.086638]  [<ffffffff8151997b>] ? _spin_unlock_bh+0x1b/0x20
<4>[46267.086712]  [<ffffffff8109d370>] ? autoremove_wake_function+0x0/0x40
<4>[46267.086785]  [<ffffffff81495e80>] ? do_ip_setsockopt+0x90/0xd80
<4>[46267.086858]  [<ffffffff8100be0e>] ? call_function_interrupt+0xe/0x20
<4>[46267.086936]  [<ffffffff8118cb10>] ? ub_slab_ptr+0x20/0x90
<4>[46267.087006]  [<ffffffff8118cb10>] ? ub_slab_ptr+0x20/0x90
<4>[46267.087081]  [<ffffffff8118f2e8>] ? kmem_cache_alloc+0xd8/0x1e0
<4>[46267.087151]  [<ffffffff81445599>] sys_sendto+0x139/0x190
<4>[46267.087229]  [<ffffffff81448c0d>] ? sock_setsockopt+0x16d/0x6f0
<4>[46267.087303]  [<ffffffff810efa47>] ? audit_syscall_entry+0x1d7/0x200
<4>[46267.087378]  [<ffffffff810ef795>] ? __audit_syscall_exit+0x265/0x290
<4>[46267.087454]  [<ffffffff81474885>] ? compat_sys_setsockopt+0x75/0x210
<4>[46267.087531]  [<ffffffff81474b5f>] compat_sys_socketcall+0x13f/0x210
<4>[46267.087607]  [<ffffffff8104dea3>] ia32_sysret+0x0/0x5
<4>[46267.087676] Code: 91 20 e2 01 75 29 48 89 de 4c 89 f7 e8 56 fa ff ff 85 c0 0f 84 68 fc ff ff 0f b6 4d c6 41 8b 45 00 e9 4d fb ff ff e8 7c 19 e9 e0 <0f> 0b eb fe f6 05 17 91 20 e2 80 74 ce 80 3d 5f 2e 00 00 00 74
<1>[46267.088023] RIP  [<ffffffffa01e00a4>] nf_nat_setup_info+0x564/0x590

Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 include/net/netfilter/nf_conntrack.h |  2 ++
 net/netfilter/nf_conntrack_core.c    | 11 +++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index 01ea6ee..492e857 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -76,6 +76,8 @@ struct nf_conn {
            plus 1 for any connection(s) we are `master' for */
 	struct nf_conntrack ct_general;
 
+	struct rcu_head rcu;
+
 	spinlock_t lock;
 
 	/* XXX should I move this to the tail ? - Y.K */
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 43549eb..40e0d61 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -198,6 +198,14 @@ clean_from_lists(struct nf_conn *ct)
 	nf_ct_remove_expectations(ct);
 }
 
+static void nf_conntrack_free_rcu(struct rcu_head *head)
+{
+	struct nf_conn *ct = container_of(head, struct nf_conn, rcu);
+
+	pr_debug("destroy_conntrack: returning ct=%p to slab\n", ct);
+	nf_conntrack_free(ct);
+}
+
 static void
 destroy_conntrack(struct nf_conntrack *nfct)
 {
@@ -236,8 +244,7 @@ destroy_conntrack(struct nf_conntrack *nfct)
 	if (ct->master)
 		nf_ct_put(ct->master);
 
-	pr_debug("destroy_conntrack: returning ct=%p to slab\n", ct);
-	nf_conntrack_free(ct);
+	call_rcu(&ct->rcu, nf_conntrack_free_rcu);
 }
 
 static void nf_ct_delete_from_lists(struct nf_conn *ct)
-- 
1.8.4.2

^ permalink raw reply related

* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
From: Hannes Frederic Sowa @ 2014-01-06 15:52 UTC (permalink / raw)
  To: François-Xavier Le Bail
  Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki Yoshifuji, Patrick McHardy
In-Reply-To: <1389016096.57230.YahooMailBasic@web125502.mail.ne1.yahoo.com>

On Mon, Jan 06, 2014 at 05:48:16AM -0800, François-Xavier Le Bail wrote:
> On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> 
> > On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote:
> > > On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> > > 
> > > > On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> > > > > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > > > > index 5d42009..65c8619 100644
> > > > > --- a/net/ipv6/icmp.c
> > > > > +++ b/net/ipv6/icmp.c
> > > > > @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> > > > >  
> > > > >  	saddr = &ipv6_hdr(skb)->daddr;
> > > > >  
> > > > > -	if (!ipv6_unicast_destination(skb))
> > > > > +	if (!ipv6_unicast_destination(skb) &&
> > > > > +	    !(net->ipv6.anycast_src_echo_reply &&
> > > > > +	      ipv6_chk_acast_addr(net, NULL, saddr)))
> > > > >  		saddr = NULL;
> > > 
> > > > I am not sure why you left out the device at ipv6_chk_acast_addr?
> > > 
> > > > IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> > > > scope global but need to check the interface for scope link.
> > > 
> > > > It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> > > > anycast on another interface which may not be checked here.
> > > 
> > > In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent.
> > 
> > A counter-example would be NOARP interfaces where we also don't speak ndisc.
> 
> Right. But if an echo request arrive on an interface which do not listen at this LL anycast, is a reply can occur ?

I thought quite the opposite with anycast + forwarding enabled, that
we would process the packet. But a quick test with this snippet showed no
problems on a tunnel router. Local input is ok, too.

#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <net/if.h>

int main(int argc, char **argv)
{
  struct ipv6_mreq mreq = {0};
  int sockfd=socket(AF_INET6, SOCK_DGRAM,0);
  inet_pton(AF_INET6, "fe80::AAAA", &mreq.ipv6mr_multiaddr);
  mreq.ipv6mr_interface = if_nametoindex("dummy0");
  setsockopt(sockfd, IPPROTO_IPV6, IPV6_JOIN_ANYCAST, &mreq, sizeof(mreq));
  pause();
}

I guess, I would have added skb->dev for the sake of defensive style
(and maybe performance, if tables get large). I have not tested that
and it's your choice (or Davids, of course). But I currently don't see
a reason why it should break. ;)

Greetings,

  Hannes

^ permalink raw reply

* Re: [patch iproute2 v2 0/2] add support for IFA_F_MANAGETEMPADDR
From: Thomas Haller @ 2014-01-06 15:41 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw
In-Reply-To: <20140104113543.GC25828@order.stressinduktion.org>

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

On Sat, 2014-01-04 at 12:35 +0100, Hannes Frederic Sowa wrote:
> On Sat, Jan 04, 2014 at 12:21:51PM +0100, Thomas Haller wrote:
> > > > >Also, I saw that NetworkManager switched to install autoconf addresses
> > > > >as /128, doesn't this break with IFA_F_MANAGETEMPADDR, as you expect a /64
> > > > >prefixlen?
> > > > 
> > > > /64 is required
> > > 
> > > Ok, currently NM seems to "violate" that as it installs autoconf addresses
> > > with 128 prefixlen, so IFA_F_MANAGETEMPADDR should not work on them.
> > > (currently observed on Fedora 20).
> > 
> > True, I noticed that too. I think that is a bug in NM to add the
> > addresses as /128. Probably, we will fix that soon.
> 
> The change could be valid. Otherwise currently NM could not correctly handle
> prefix information in RAs in some cases:
> 
> It is possible to let the client generate an autonomously address in a
> prefix which is actually not on-link (L=0). Kernel would automatically
> create prefix route by mistake, if NM tries to install such an address
> with /64 prefix. This does not happen if the prefix address uses 128
> prefixlen.
> 
> Would be great to have feedback on this, as this could be easily solved by an
> additional ifa_flag.
> 
> Greetings,
> 
>   Hannes
> 


Hi Hannes,


good point. I think, the user-space application (NetworkManager) should
add the autoconf addresses as /64. But the kernel should not generate
any routes in that case. It's up to the application to add them
(depending on the on-link flag).


What do you think about something like:



diff --git i/net/ipv6/addrconf.c w/net/ipv6/addrconf.c
index 6c16345..5a4c382 100644
--- i/net/ipv6/addrconf.c
+++ w/net/ipv6/addrconf.c
@@ -2433,8 +2433,11 @@ static int inet6_addr_add(struct net *net, int ifindex,
 			    valid_lft, prefered_lft);
 
 	if (!IS_ERR(ifp)) {
-		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
-				      expires, flags);
+		if (ifa_flags & IFA_F_MANAGETEMPADDR == 0) {
+			addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
+					      expires, flags);
+		}
+
 		/*
 		 * Note that section 3.1 of RFC 4429 indicates
 		 * that the Optimistic flag should not be set for


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related

* Re: [PATCH 03/12] netfilter: nfnetlink_queue: enable UID/GID socket info retrieval
From: Eric Dumazet @ 2014-01-06 15:32 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Valentina Giusti; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <1388963586-5049-4-git-send-email-pablo@netfilter.org>

On Mon, 2014-01-06 at 00:12 +0100, Pablo Neira Ayuso wrote:
> From: Valentina Giusti <valentina.giusti@bmw-carit.de>
> 
> Thanks to commits 41063e9 (ipv4: Early TCP socket demux) and 421b388
> (udp: ipv4: Add udp early demux) it is now possible to parse UID and
> GID socket info also for incoming TCP and UDP connections. Having
> this info available, it is convenient to let NFQUEUE parse it in
> order to improve and refine the traffic analysis in userspace.
> 
> Signed-off-by: Valentina Giusti <valentina.giusti@bmw-carit.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/uapi/linux/netfilter/nfnetlink_queue.h |    5 +++-
>  net/netfilter/nfnetlink_queue_core.c           |   34 ++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h
> index 0132bad..8dd819e 100644
> --- a/include/uapi/linux/netfilter/nfnetlink_queue.h
> +++ b/include/uapi/linux/netfilter/nfnetlink_queue.h
> @@ -47,6 +47,8 @@ enum nfqnl_attr_type {
>  	NFQA_CAP_LEN,			/* __u32 length of captured packet */
>  	NFQA_SKB_INFO,			/* __u32 skb meta information */
>  	NFQA_EXP,			/* nf_conntrack_netlink.h */
> +	NFQA_UID,			/* __u32 sk uid */
> +	NFQA_GID,			/* __u32 sk gid */
>  
>  	__NFQA_MAX
>  };
> @@ -99,7 +101,8 @@ enum nfqnl_attr_config {
>  #define NFQA_CFG_F_FAIL_OPEN			(1 << 0)
>  #define NFQA_CFG_F_CONNTRACK			(1 << 1)
>  #define NFQA_CFG_F_GSO				(1 << 2)
> -#define NFQA_CFG_F_MAX				(1 << 3)
> +#define NFQA_CFG_F_UID_GID			(1 << 3)
> +#define NFQA_CFG_F_MAX				(1 << 4)
>  
>  /* flags for NFQA_SKB_INFO */
>  /* packet appears to have wrong checksums, but they are ok */
> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
> index 21258cf..d3cf12b 100644
> --- a/net/netfilter/nfnetlink_queue_core.c
> +++ b/net/netfilter/nfnetlink_queue_core.c
> @@ -297,6 +297,31 @@ nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet,
>  	return flags ? nla_put_be32(nlskb, NFQA_SKB_INFO, htonl(flags)) : 0;
>  }
>  
> +static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk)
> +{
> +	const struct cred *cred;
> +
> +	if (sk->sk_state == TCP_TIME_WAIT)
> +		return 0;
> +

net/netfilter/nfnetlink_queue_core.c: In function 'nfqnl_put_sk_uidgid':
net/netfilter/nfnetlink_queue_core.c:304:35: error: 'TCP_TIME_WAIT' undeclared (first use in this function)
net/netfilter/nfnetlink_queue_core.c:304:35: note: each undeclared identifier is reported only once for each function it appears in
make[3]: *** [net/netfilter/nfnetlink_queue_core.o] Error 1

Fix is obvious (#include <net/tcp_states.h>), but I have to run...




^ permalink raw reply

* Re: [PATCH net 2/2] net: core: explicitly select a txq before doing l2 forwarding
From: Neil Horman @ 2014-01-06 15:29 UTC (permalink / raw)
  To: John Fastabend
  Cc: mst, e1000-devel, netdev, Jason Wang, linux-kernel,
	John Fastabend, davem
In-Reply-To: <52CAC671.4040208@gmail.com>

On Mon, Jan 06, 2014 at 07:06:25AM -0800, John Fastabend wrote:
> On 01/06/2014 04:42 AM, Neil Horman wrote:
> >On Mon, Jan 06, 2014 at 11:21:07AM +0800, Jason Wang wrote:
> >>Currently, the tx queue were selected implicitly in ndo_dfwd_start_xmit(). The
> >>will cause several issues:
> >>
> >>- NETIF_F_LLTX was forced for macvlan device in this case which lead extra lock
> >>   contention.
> >>- dev_hard_start_xmit() was called with NULL txq which bypasses the net device
> >>   watchdog
> >>- dev_hard_start_xmit() does not check txq everywhere which will lead a crash
> >>   when tso is disabled for lower device.
> >>
> >>Fix this by explicitly introducing a select queue method just for l2 forwarding
> >>offload (ndo_dfwd_select_queue), and introducing dfwd_direct_xmit() to do the
> >>queue selecting and transmitting for l2 forwarding.
> >>
> >>With this fixes, NETIF_F_LLTX could be preserved for macvlan and there's no need
> >>to check txq against NULL in dev_hard_start_xmit().
> >>
> >>In the future, it was also required for macvtap l2 forwarding support since it
> >>provides a necessary synchronization method.
> >>
> >>Cc: John Fastabend <john.r.fastabend@intel.com>
> >>Cc: Neil Horman <nhorman@tuxdriver.com>
> >>Cc: e1000-devel@lists.sourceforge.net
> >>Signed-off-by: Jason Wang <jasowang@redhat.com>
> >
> >Instead of creating another operation here to do special queue selection, why
> >not just have ndo_dfwd_start_xmit include a pointer to a pointer in its argument
> >list, so it can pass the txq it used back to the caller (dev_hard_start_xmit)?
> >ndo_dfwd_start_xmit already knows which queue set to pick from (since their
> >reserved for the device doing the transmitting).  It seems more clear to me than
> >creating a new netdevice operation.
> >
> >As for the crash issue, I'm not sure what you mean.  Where in
> >dev_hard_start_xmit would we need to check txq that we're not currently, and
> >what crash results?
> >
> >Also, can you elaborate on what you mean by additional lock contention?  What
> >contention do you see that goes above and beyond the normal locking required by
> >txq access?  I suppose its extra locking above and beyond in the macvtap case,
> >where you would otherwise never hit hardware, but that not the only use case,
> >and I think the solution there is likely to add some code in the macvlan feature
> >set handler so that NETIF_F_LLTX is cleared if you disable the hardware
> >forwarding acceleration via ethtool.
> >
> 
> NETIF_F_LLTX is cleared in macvlan_open() which should be used in the
> macvtap case.
> 
Thats right, since accelerated hardware tx queue doesn't participate in the
network stack queue locking, the upper device needs to do it.

Thanks!
Neil

>           if (lowerdev->features & NETIF_F_HW_L2FW_DOFFLOAD) {
>                   vlan->fwd_priv =
> 
> lowerdev->netdev_ops->ndo_dfwd_add_station(lowerdev, dev);
> 
>                   /* If we get a NULL pointer back, or if we get an error
>                    * then we should just fall through to the non
> accelerated path
>                    */
>                   if (IS_ERR_OR_NULL(vlan->fwd_priv)) {
>                           vlan->fwd_priv = NULL;
>                   } else {
>                           dev->features &= ~NETIF_F_LLTX;
>                           return 0;
>                   }
>           }
> 
> Thanks,
> John
> 
> 
> -- 
> John Fastabend         Intel Corporation
> 

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH 13/21] mac8011: slight optimization of addr compare
From: Johannes Berg @ 2014-01-06 15:21 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: John W. Linville, David S. Miller, linux-wireless, Netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <52B7C5DA.9090307@huawei.com>

On Mon, 2013-12-23 at 13:10 +0800, Ding Tianhong wrote:
> Use the recently added and possibly more efficient
> ether_addr_equal_unaligned to instead of memcmp.

> -				if (memcmp(local->hw.wiphy->addresses[i].addr,
> -					   sdata->vif.addr, ETH_ALEN) == 0) {
> +				if (ether_addr_equal_unaligned(local->hw.wiphy->addresses[i].addr, sdata->vif.addr)) {

This clearly should have some linebreaks.

This code also doesn't care all that much about efficiency, so not sure
it's worth it at all.

johannes

^ permalink raw reply

* Re: [PATCH 4/11] use ether_addr_equal_64bits
From: Eric Dumazet @ 2014-01-06 15:18 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Emmanuel Grumbach, Johannes Berg, Henrique de Moraes Holschuh,
	Julia Lawall, kernel-janitors@vger.kernel.org, Emmanuel Grumbach,
	Intel Linux Wireless, John W. Linville, linux-wireless,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdXZQO7jj+V2mMyyLgDoz6iSLJ25+55Yow-Acb1=tE5sMg@mail.gmail.com>

On Mon, 2014-01-06 at 10:24 +0100, Geert Uytterhoeven wrote:

> Thanks to this discussion, my eye fell on:
> 
> static inline unsigned compare_ether_addr(const u8 *addr1, const u8 *addr2)
> {
>         const u16 *a = (const u16 *) addr1;
>         const u16 *b = (const u16 *) addr2;
> 
>         BUILD_BUG_ON(ETH_ALEN != 6);
>         return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) != 0;
> }
> 
> What if addr1 or addr2 are odd, and this is running on an architecture that
> doesn't support unaligned accesses at all?? Have we been lucky forever?

This function always had the guarantee of u16 alignment.

No protocol ever included an ether address at an odd alignment, and
drivers always make sure a frame is at least 2-byte aligned.

Its kind of obvious for networking people, Stephen did not mention this
property in commit 360ac8e2f1a38c34

^ permalink raw reply

* Re: [PATCH v2 5/5] alx: add stats to ethtool
From: Johannes Berg @ 2014-01-06 15:16 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: davem, bhutchings, netdev
In-Reply-To: <20140106145629.GA7886@kria>

On Mon, 2014-01-06 at 15:56 +0100, Sabrina Dubroca wrote:

> > I was more worried about type mismatches. That's not really a concern
> > with u64 since that's the largest type that really makes sense here, but
> > if the type of some variables changed vs. the ethtool type u64...
> 
> Add "all stats fields must be u64" to the comment before the struct?

I guess that's OK. I can live with either of these solutions really, I
have no plans to change this driver ever again (though I might need WOL
soon ... *sigh*)

johannes

^ 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