Netdev List
 help / color / mirror / Atom feed
* Re: [net-next 05/15] i40e: add a comment on barrier and fix panic on reset
From: David Miller @ 2014-01-09 20:12 UTC (permalink / raw)
  To: gregory.v.rose
  Cc: sergei.shtylyov, jeffrey.t.kirsher, netdev, gospo, sassmann,
	mitch.a.williams, jesse.brandeburg
In-Reply-To: <20140109091735.00002cf0@unknown>

From: Greg Rose <gregory.v.rose@intel.com>
Date: Thu, 9 Jan 2014 09:17:35 -0800

> Yes, it's apparent that two different internal patches were compressed
> together.  If its unacceptable to do this then I'll speak to Jeff about
> splitting them.

I won't reject the entire pull request on account of this, but please
don't do this in the future.

^ permalink raw reply

* Re: [PATCH net-next v3 1/9] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2014-01-09 19:53 UTC (permalink / raw)
  To: Wei Liu
  Cc: ian.campbell, xen-devel, netdev, linux-kernel, jonathan.davies,
	Roger Pau Monné, David Vrabel
In-Reply-To: <20140109153010.GE12164@zion.uk.xensource.com>

On 09/01/14 15:30, Wei Liu wrote:
> On Wed, Jan 08, 2014 at 12:10:10AM +0000, Zoltan Kiss wrote:
>> v3:
>> - fix comment in xenvif_tx_dealloc_action()
>> - call unmap hypercall directly instead of gnttab_unmap_refs(), which does
>>    unnecessary m2p_override. Also remove pages_to_[un]map members
>
> Is it worthy to have another function call
> gnttab_unmap_refs_no_m2p_override in Xen core driver, or just add a
> parameter to control wether we need to touch m2p_override? I *think* it
> will benefit block driver as well?
>
> (CC Roger and David for input)

Yep, it worth, but let's make it a different patch

>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -771,6 +771,19 @@ static struct page *xenvif_alloc_page(struct xenvif *vif,
>>   	return page;
>>   }
>>
>> +static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
>> +	       struct xen_netif_tx_request *txp,
>> +	       struct gnttab_map_grant_ref *gop)
>
> Indentation.
I fixed it and the later ones up, hopefully I haven't missed anything.

>
>> +
>> +	spin_lock_irqsave(&vif->dealloc_lock, flags);
>> +	do {
>> +		pending_idx = ubuf->desc;
>> +		ubuf = (struct ubuf_info *) ubuf->ctx;
>> +		index = pending_index(vif->dealloc_prod);
>> +		vif->dealloc_ring[index] = pending_idx;
>> +		/* Sync with xenvif_tx_action_dealloc:
>
> xenvif_tx_dealloc_action I suppose.
Yes.

>> +			/* Already unmapped? */
>> +			if (vif->grant_tx_handle[pending_idx] ==
>> +				NETBACK_INVALID_HANDLE) {
>> +				netdev_err(vif->dev,
>> +					"Trying to unmap invalid handle! "
>> +					"pending_idx: %x\n", pending_idx);
>> +				continue;
>
> You seemed to miss the BUG_ON we discussed?
>
> See thread starting <52AF1A84.3090304@citrix.com>.
Indeed, despite I wrote it in the version history :)

^ permalink raw reply

* Re: [net-next 3/7] ixgbe: Use static inlines instead of macros
From: David Miller @ 2014-01-09 19:39 UTC (permalink / raw)
  To: mark.d.rustad; +Cc: sfeldma, aaron.f.brown, netdev, gospo, sassmann
In-Reply-To: <2C8FA848-3185-43BA-BDD7-955B0C7DD835@intel.com>

From: "Rustad, Mark D" <mark.d.rustad@intel.com>
Date: Thu, 9 Jan 2014 17:34:18 +0000

> On Jan 8, 2014, at 12:47 AM, Scott Feldman <sfeldma@cumulusnetworks.com> wrote:
> 
>> 
>> On Jan 7, 2014, at 11:40 PM, Aaron Brown <aaron.f.brown@intel.com> wrote:
>> 
>>> From: Mark Rustad <mark.d.rustad@intel.com>
>>> 
>>> -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg)))
>>> +static inline void IXGBE_WRITE_REG(struct ixgbe_hw *hw, u32 reg, u32 value)
>> 
>> Bummer, now you have a all-caps func name.
> 
> Agreed, but this is actually a fairly common condition among drivers that used to use macros. It isn't perfect, but at least it is moving in the right direction. I'd rather leave the case change for a later patch series that does only that or has some reason to touch all of the register access sites.
> 
> At least the new accessor I introduced is lower case. :-)

Please address this feedback, all caps function names are really not
appropriate.

^ permalink raw reply

* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael Dalton @ 2014-01-09 19:33 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <20140109132529.GB2712@redhat.com>

Hi Michael,

Your improvements (code changes, more consistent naming, and use of
256-byte alignment only) all sound good to me. I will get started on a v3
patchset in conformance with your recommendations after sorting out
what we want to do with the debugfs/sysfs issues. I will followup soon on
the thread for patch 4/4 so we can close on what changes are needed for
debugfs/sysfs. Thanks!

Best,

Mike

^ permalink raw reply

* Re: [PATCH -next] ip_tunnel: fix sparse non static symbol warning
From: David Miller @ 2014-01-09 19:33 UTC (permalink / raw)
  To: weiyj.lk; +Cc: kuznet, jmorris, yoshfuji, kaber, therbert, yongjun_wei, netdev
In-Reply-To: <CAPgLHd__qSufRW4YE63SocWV1gcgzBCBX1bdy+WcJ7v8zhE8qw@mail.gmail.com>

From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Wed, 8 Jan 2014 21:59:30 +0800

> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> Fixes the following sparse warning:
> 
> net/ipv4/ip_tunnel.c:116:18: warning:
>  symbol 'tunnel_dst_check' was not declared. Should it be static?
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Applied.

^ permalink raw reply

* Re: [PATCH -next] openvswitch: Use kmem_cache_free() instead of kfree()
From: David Miller @ 2014-01-09 19:27 UTC (permalink / raw)
  To: jesse; +Cc: weiyj.lk, pshelar, yongjun_wei, dev, netdev
In-Reply-To: <CAEP_g=9HVB0UkajKesyC8Dkj=0ts3kExbhhHKj2wrjx9RymEog@mail.gmail.com>

From: Jesse Gross <jesse@nicira.com>
Date: Wed, 8 Jan 2014 09:17:11 -0500

> On Wed, Jan 8, 2014 at 5:13 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> memory allocated by kmem_cache_alloc() should be freed using
>> kmem_cache_free(), not kfree().
>>
>> Fixes: e298e5057006 ('openvswitch: Per cpu flow stats.')
>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> Good catch, thanks.
> 
> I'll let David pick this up directly since I just flushed my patch queue.
> 
> Acked-by: Jesse Gross <jesse@nicira.com>

Applied, thanks.

^ permalink raw reply

* RE: [PATCH net] bnx2x: fix DMA unmapping of TSO split BDs
From: Dmitry Kravkov @ 2014-01-09 19:23 UTC (permalink / raw)
  To: Eric Dumazet, Michal Schmidt
  Cc: netdev@vger.kernel.org, Ariel Elior, Yuval Mintz, David Miller
In-Reply-To: <1389289923.31367.39.camel@edumazet-glaptop2.roam.corp.google.com>

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Thursday, January 09, 2014 7:52 PM
> To: Michal Schmidt
> Cc: netdev@vger.kernel.org; Ariel Elior; Yuval Mintz; Dmitry Kravkov; David
> Miller
> Subject: Re: [PATCH net] bnx2x: fix DMA unmapping of TSO split BDs
> 
> On Thu, 2014-01-09 at 14:36 +0100, Michal Schmidt wrote:
> > bnx2x triggers warnings with CONFIG_DMA_API_DEBUG=y:
> >
> >   WARNING: CPU: 0 PID: 2253 at lib/dma-debug.c:887
> check_unmap+0xf8/0x920()
> >   bnx2x 0000:28:00.0: DMA-API: device driver frees DMA memory with
> >   different size [device address=0x00000000da2b389e] [map size=1490
> bytes]
> >   [unmap size=66 bytes]
> >
> > The reason is that bnx2x splits a TSO BD into two BDs (headers + data)
> > using one DMA mapping for both, but it uses only the length of the
> > first BD when unmapping.
> >
> > This patch fixes the bug by unmapping the whole length of the two BDs.
> >
> > Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> > ---
> 
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> 

Thanks you, Guys.

Acked-by: Dmitry Kravkov <dmitry@broadcom.com>


^ permalink raw reply

* Re: [PATCH net-next 2/2] include/uapi/linux/xfrm.h: Pack struct xfrm_usersa_info
From: Ben Hutchings @ 2014-01-09 18:58 UTC (permalink / raw)
  To: Fan Du; +Cc: steffen.klassert, davem, stephen, dev, netdev
In-Reply-To: <52CE5CBA.9000200@windriver.com>

On Thu, 2014-01-09 at 16:24 +0800, Fan Du wrote:
> 
> On 2014年01月09日 04:33, Ben Hutchings wrote:
> > On Tue, 2014-01-07 at 14:48 +0800, Fan Du wrote:
> >> Otherwise 64bits kernel has sizeof(struct xfrm_usersa_info) 224 bytes,
> >> while 32bits compiled iproute2 see the same structure as 220 bytes, which
> >> leading deficit xfrm sa, in turn broken IPsec connectivity.
> >>
> >> Fix this by packing the structure.
> >>
> >> Signed-off-by: Fan Du<fan.du@windriver.com>
> >> ---
> >>   include/uapi/linux/xfrm.h |    2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> >> index 470bfae..61460c4 100644
> >> --- a/include/uapi/linux/xfrm.h
> >> +++ b/include/uapi/linux/xfrm.h
> >> @@ -366,7 +366,7 @@ struct xfrm_usersa_info {
> >>   #define XFRM_STATE_AF_UNSPEC	32
> >>   #define XFRM_STATE_ALIGN4	64
> >>   #define XFRM_STATE_ESN		128
> >> -};
> >> +} __attribute__((packed));
> >>
> >>   #define XFRM_SA_XFLAG_DONT_ENCAP_DSCP	1
> >>
> >
> > That change will make access to the structure very slow on some
> > architectures, and I suspect it will cause other compatibility problems.
> >
> > I think the right thing to do is to reduce the minimum length of the
> > structure in the netlink policy so that padding at the end is not
> > required.
> 
> Could you please be more specific about this? Thanks.
[...]

Each netlink message and attribute has a specified minimum size, and
messages are rejected if the size provided by the sender is too small.
The minimum size is normally the same as the structure size, but doesn't
have to be.

Something like this might work:

--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2266,8 +2266,15 @@ static int xfrm_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 
 #define XMSGSIZE(type) sizeof(struct type)
 
+/* Padding at the end of struct xfrm_usersa_info differs between
+ * architectures so for 32/64-bit compat we don't require padding
+ */
+#define XMSGSIZE_XFRM_USERSA_INFO				\
+	(offsetof(struct xfrm_usersa_info, flags) +		\
+	 sizeof(((struct xfrm_usersa_info *)NULL)->flags))
+
 static const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
-	[XFRM_MSG_NEWSA       - XFRM_MSG_BASE] = XMSGSIZE(xfrm_usersa_info),
+	[XFRM_MSG_NEWSA       - XFRM_MSG_BASE] = XMSGSIZE_XFRM_USERSA_INFO,
 	[XFRM_MSG_DELSA       - XFRM_MSG_BASE] = XMSGSIZE(xfrm_usersa_id),
 	[XFRM_MSG_GETSA       - XFRM_MSG_BASE] = XMSGSIZE(xfrm_usersa_id),
 	[XFRM_MSG_NEWPOLICY   - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_info),
@@ -2277,7 +2284,7 @@ static const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
 	[XFRM_MSG_ACQUIRE     - XFRM_MSG_BASE] = XMSGSIZE(xfrm_user_acquire),
 	[XFRM_MSG_EXPIRE      - XFRM_MSG_BASE] = XMSGSIZE(xfrm_user_expire),
 	[XFRM_MSG_UPDPOLICY   - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_info),
-	[XFRM_MSG_UPDSA       - XFRM_MSG_BASE] = XMSGSIZE(xfrm_usersa_info),
+	[XFRM_MSG_UPDSA       - XFRM_MSG_BASE] = XMSGSIZE_XFRM_USERSA_INFO,
 	[XFRM_MSG_POLEXPIRE   - XFRM_MSG_BASE] = XMSGSIZE(xfrm_user_polexpire),
 	[XFRM_MSG_FLUSHSA     - XFRM_MSG_BASE] = XMSGSIZE(xfrm_usersa_flush),
 	[XFRM_MSG_FLUSHPOLICY - XFRM_MSG_BASE] = 0,
@@ -2292,7 +2299,7 @@ static const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
 #undef XMSGSIZE
 
 static const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
-	[XFRMA_SA]		= { .len = sizeof(struct xfrm_usersa_info)},
+	[XFRMA_SA]		= { .len = XMSGSIZE_XFRM_USERSA_INFO },
 	[XFRMA_POLICY]		= { .len = sizeof(struct xfrm_userpolicy_info)},
 	[XFRMA_LASTUSED]	= { .type = NLA_U64},
 	[XFRMA_ALG_AUTH_TRUNC]	= { .len = sizeof(struct xfrm_algo_auth)},
---

But beware that if the structure is copied in and then copied back out
to userland then the padding will need to be explicitly cleared.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH v4 4/5] alx: add alx_get_stats64 operation
From: Ben Hutchings @ 2014-01-09 18:47 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: davem, johannes, stephen, netdev
In-Reply-To: <1389258571-10083-5-git-send-email-sd@queasysnail.net>

On Thu, 2014-01-09 at 10:09 +0100, Sabrina Dubroca wrote:
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>

Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>

> ---
> 
> I've modified the assignements of hw stats to netstats here as Ben asked in v3.
> 
> The other atheros drivers use the same code as v3. Should I modify
> them as well in another patch?

That seems like a good thing to do.

Ben.

> 
>  drivers/net/ethernet/atheros/alx/alx.h  |  3 ++
>  drivers/net/ethernet/atheros/alx/main.c | 50 +++++++++++++++++++++++++++++++++
>  2 files changed, 53 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..e92ffd6 100644
> --- a/drivers/net/ethernet/atheros/alx/main.c
> +++ b/drivers/net/ethernet/atheros/alx/main.c
> @@ -1166,10 +1166,60 @@ 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_bytes   = hw_stats->tx_byte_cnt;
> +	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 +
> +				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 +
> +				hw_stats->rx_ov_rxf;
> +
> +	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_dropped       = hw_stats->rx_ov_rrd;
> +
> +	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;
> +
> +	net_stats->tx_packets = hw_stats->tx_ok + net_stats->tx_errors;
> +	net_stats->rx_packets = hw_stats->rx_ok + net_stats->rx_errors;
> +
> +	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,

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH net-next] net/mlx4_en: call gro handler for encapsulated frames
From: Eric Dumazet @ 2014-01-09 18:30 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Amir Vadai, Or Gerlitz, Jerry Chu

From: Eric Dumazet <edumazet@google.com>

In order to use the native GRO handling of encapsulated protocols on
mlx4, we need to call napi_gro_receive() instead of netif_receive_skb()
unless busy polling is in action.

While we are at it, rename mlx4_en_cq_ll_polling() to
mlx4_en_cq_busy_polling()

Tested with GRE tunnel : GRO aggregation is now performed on the
ethernet device instead of being done later on gre device.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Amir Vadai <amirv@mellanox.com>
Cc: Jerry Chu <hkchu@google.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_rx.c   |    8 +++++---
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |    4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 3b66f26ba049..890922c1c8ee 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -724,7 +724,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 				 * - not an IP fragment
 				 * - no LLS polling in progress
 				 */
-				if (!mlx4_en_cq_ll_polling(cq) &&
+				if (!mlx4_en_cq_busy_polling(cq) &&
 				    (dev->features & NETIF_F_GRO)) {
 					struct sk_buff *gro_skb = napi_get_frags(&cq->napi);
 					if (!gro_skb)
@@ -816,8 +816,10 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 
 		skb_mark_napi_id(skb, &cq->napi);
 
-		/* Push it up the stack */
-		netif_receive_skb(skb);
+		if (!mlx4_en_cq_busy_polling(cq))
+			napi_gro_receive(&cq->napi, skb);
+		else
+			netif_receive_skb(skb);
 
 next:
 		for (nr = 0; nr < priv->num_frags; nr++)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 2f1e200f2e4c..fe7bdfebf353 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -661,7 +661,7 @@ static inline bool mlx4_en_cq_unlock_poll(struct mlx4_en_cq *cq)
 }
 
 /* true if a socket is polling, even if it did not get the lock */
-static inline bool mlx4_en_cq_ll_polling(struct mlx4_en_cq *cq)
+static inline bool mlx4_en_cq_busy_polling(struct mlx4_en_cq *cq)
 {
 	WARN_ON(!(cq->state & MLX4_CQ_LOCKED));
 	return cq->state & CQ_USER_PEND;
@@ -691,7 +691,7 @@ static inline bool mlx4_en_cq_unlock_poll(struct mlx4_en_cq *cq)
 	return false;
 }
 
-static inline bool mlx4_en_cq_ll_polling(struct mlx4_en_cq *cq)
+static inline bool mlx4_en_cq_busy_polling(struct mlx4_en_cq *cq)
 {
 	return false;
 }

^ permalink raw reply related

* Re: [Xen-devel] [PATCH net-next v3 1/9] xen-netback: Introduce TX grant map definitions
From: David Vrabel @ 2014-01-09 18:23 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Wei Liu, jonathan.davies, Ian Campbell, netdev, linux-kernel,
	Zoltan Kiss, xen-devel, Roger Pau Monné,
	Konrad Rzeszutek Wilk
In-Reply-To: <alpine.DEB.2.02.1401091759100.21510@kaball.uk.xensource.com>

On 09/01/14 18:09, Stefano Stabellini wrote:
> 
> I agree, but I think that we have different views on the use case.
> To me the m2p_override use case is "everywhere an mfn_to_pfn translation
> is required", that unfortunately is potentially everywhere at this time.

mfn_to_pfn() cannot be made to work correctly with foreign MFNs.  It's a
fundamentally unsolvable problem.

IMO, the only sensible use of the m2p_override is to cause mfn_to_pfn()
to BUG() if a foreign MFN is used.

David

^ permalink raw reply

* [RFC Patch net-next 4/4] net_sched: make ingress qdisc lockless
From: Cong Wang @ 2014-01-09 18:19 UTC (permalink / raw)
  To: netdev
  Cc: Cong Wang, John Fastabend, Eric Dumazet, David S. Miller,
	Jamal Hadi Salim
In-Reply-To: <1389291593-2494-1-git-send-email-xiyou.wangcong@gmail.com>

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/core/dev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index ce01847..e357d05 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3376,10 +3376,8 @@ static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
 
 	q = rxq->qdisc;
 	if (q != &noop_qdisc) {
-		spin_lock(qdisc_lock(q));
 		if (likely(!test_bit(__QDISC_STATE_DEACTIVATED, &q->state)))
 			result = qdisc_enqueue_root(skb, q);
-		spin_unlock(qdisc_lock(q));
 	}
 
 	return result;
-- 
1.8.3.1

^ permalink raw reply related

* [RFC Patch net-next 3/4] net_sched: use RCU for tc actions traverse
From: Cong Wang @ 2014-01-09 18:19 UTC (permalink / raw)
  To: netdev
  Cc: Cong Wang, John Fastabend, Eric Dumazet, David S. Miller,
	Jamal Hadi Salim
In-Reply-To: <1389291593-2494-1-git-send-email-xiyou.wangcong@gmail.com>

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/act_api.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f63e146..e3c655e 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -351,7 +351,7 @@ int tcf_action_exec(struct sk_buff *skb, const struct list_head *actions,
 		ret = TC_ACT_OK;
 		goto exec_done;
 	}
-	list_for_each_entry(a, actions, list) {
+	list_for_each_entry_rcu(a, actions, list) {
 repeat:
 		ret = a->ops->act(skb, a, res);
 		if (TC_MUNGED & skb->tc_verd) {
@@ -372,11 +372,13 @@ EXPORT_SYMBOL(tcf_action_exec);
 void tcf_action_destroy(struct list_head *actions, int bind)
 {
 	struct tc_action *a, *tmp;
+	LIST_HEAD(list);
+	list_splice_init_rcu(actions, &list, synchronize_rcu);
 
-	list_for_each_entry_safe(a, tmp, actions, list) {
+	list_for_each_entry_safe(a, tmp, &list, list) {
 		if (a->ops->cleanup(a, bind) == ACT_P_DELETED)
 			module_put(a->ops->owner);
-		list_del(&a->list);
+		list_del_rcu(&a->list);
 		kfree(a);
 	}
 }
@@ -420,7 +422,7 @@ tcf_action_dump(struct sk_buff *skb, struct list_head *actions, int bind, int re
 	int err = -EINVAL;
 	struct nlattr *nest;
 
-	list_for_each_entry(a, actions, list) {
+	list_for_each_entry_rcu(a, actions, list) {
 		nest = nla_nest_start(skb, a->order);
 		if (nest == NULL)
 			goto nla_put_failure;
@@ -542,7 +544,7 @@ int tcf_action_init(struct net *net, struct nlattr *nla,
 			goto err;
 		}
 		act->order = i;
-		list_add_tail(&act->list, actions);
+		list_add_tail_rcu(&act->list, actions);
 	}
 	return 0;
 
-- 
1.8.3.1

^ permalink raw reply related

* [RFC Patch net-next 2/4] net_sched: avoid holding qdisc lock for filters
From: Cong Wang @ 2014-01-09 18:19 UTC (permalink / raw)
  To: netdev
  Cc: Cong Wang, John Fastabend, Eric Dumazet, David S. Miller,
	Jamal Hadi Salim
In-Reply-To: <1389291593-2494-1-git-send-email-xiyou.wangcong@gmail.com>

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/pkt_cls.h     | 25 ++++---------------------
 include/net/sch_generic.h |  5 +++--
 net/sched/cls_api.c       | 12 +++++-------
 3 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 50ea079..9afb15b 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -18,26 +18,9 @@ int register_tcf_proto_ops(struct tcf_proto_ops *ops);
 int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
 
 static inline unsigned long
-__cls_set_class(unsigned long *clp, unsigned long cl)
+cls_set_class(unsigned long *clp, unsigned long cl)
 {
-	unsigned long old_cl;
- 
-	old_cl = *clp;
-	*clp = cl;
-	return old_cl;
-}
-
-static inline unsigned long
-cls_set_class(struct tcf_proto *tp, unsigned long *clp, 
-	unsigned long cl)
-{
-	unsigned long old_cl;
-	
-	tcf_tree_lock(tp);
-	old_cl = __cls_set_class(clp, cl);
-	tcf_tree_unlock(tp);
- 
-	return old_cl;
+	return xchg(clp, cl);
 }
 
 static inline void
@@ -46,7 +29,7 @@ tcf_bind_filter(struct tcf_proto *tp, struct tcf_result *r, unsigned long base)
 	unsigned long cl;
 
 	cl = tp->q->ops->cl_ops->bind_tcf(tp->q, base, r->classid);
-	cl = cls_set_class(tp, &r->class, cl);
+	cl = cls_set_class(&r->class, cl);
 	if (cl)
 		tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
 }
@@ -56,7 +39,7 @@ tcf_unbind_filter(struct tcf_proto *tp, struct tcf_result *r)
 {
 	unsigned long cl;
 
-	if ((cl = __cls_set_class(&r->class, 0)) != 0)
+	if ((cl = cls_set_class(&r->class, 0)) != 0)
 		tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
 }
 
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 97123cc..000ce54 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -215,6 +215,7 @@ struct tcf_proto_ops {
 
 struct tcf_proto {
 	/* Fast access part */
+	spinlock_t		lock;
 	struct list_head	head;
 	void			*root;
 	int			(*classify)(struct sk_buff *,
@@ -312,8 +313,8 @@ static inline void sch_tree_unlock(const struct Qdisc *q)
 	spin_unlock_bh(qdisc_root_sleeping_lock(q));
 }
 
-#define tcf_tree_lock(tp)	sch_tree_lock((tp)->q)
-#define tcf_tree_unlock(tp)	sch_tree_unlock((tp)->q)
+#define tcf_tree_lock(tp)	spin_lock_bh(&(tp)->lock);
+#define tcf_tree_unlock(tp)	spin_unlock_bh(&(tp)->lock);
 
 extern struct Qdisc noop_qdisc;
 extern struct Qdisc_ops noop_qdisc_ops;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index bcc9987..9ba2804 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -117,7 +117,6 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 {
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tca[TCA_MAX + 1];
-	spinlock_t *root_lock;
 	struct tcmsg *t;
 	u32 protocol;
 	u32 prio;
@@ -215,8 +214,6 @@ replay:
 	}
 	rcu_read_unlock();
 
-	root_lock = qdisc_root_sleeping_lock(q);
-
 	if ((tp = res) == NULL) {
 		/* Proto-tcf does not exist, create new one */
 
@@ -236,6 +233,7 @@ replay:
 		if (tp == NULL)
 			goto errout;
 		INIT_LIST_HEAD(&tp->head);
+		spin_lock_init(&tp->lock);
 		err = -ENOENT;
 		tp_ops = tcf_proto_lookup_ops(tca[TCA_KIND]);
 		if (tp_ops == NULL) {
@@ -287,9 +285,9 @@ replay:
 
 	if (fh == 0) {
 		if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) {
-			spin_lock_bh(root_lock);
+			tcf_tree_lock(tp);
 			list_del_rcu(&tp->head);
-			spin_unlock_bh(root_lock);
+			tcf_tree_unlock(tp);
 
 			tfilter_notify(net, skb, n, tp, fh, RTM_DELTFILTER);
 			tcf_destroy(tp);
@@ -328,9 +326,9 @@ replay:
 	err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh);
 	if (err == 0) {
 		if (tp_created) {
-			spin_lock_bh(root_lock);
+			tcf_tree_lock(tp);
 			list_add_rcu(&tp->head, chain);
-			spin_unlock_bh(root_lock);
+			tcf_tree_unlock(tp);
 		}
 		tfilter_notify(net, skb, n, tp, fh, RTM_NEWTFILTER);
 	} else {
-- 
1.8.3.1

^ permalink raw reply related

* [RFC Patch net-next 1/4] net_sched: switch filter list to list_head
From: Cong Wang @ 2014-01-09 18:19 UTC (permalink / raw)
  To: netdev
  Cc: Cong Wang, John Fastabend, Eric Dumazet, David S. Miller,
	Jamal Hadi Salim
In-Reply-To: <1389291593-2494-1-git-send-email-xiyou.wangcong@gmail.com>

So that it could be potentially traversed with RCU.

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/pkt_sched.h   |  4 ++--
 include/net/sch_generic.h |  9 ++++++---
 net/sched/cls_api.c       | 36 ++++++++++++++++++++++++------------
 net/sched/sch_api.c       | 35 ++++++++++++++++++++++-------------
 net/sched/sch_atm.c       | 14 +++++++-------
 net/sched/sch_cbq.c       |  9 +++++----
 net/sched/sch_choke.c     | 11 ++++++-----
 net/sched/sch_drr.c       |  7 ++++---
 net/sched/sch_dsmark.c    |  7 ++++---
 net/sched/sch_fq_codel.c  |  9 +++++----
 net/sched/sch_hfsc.c      | 15 +++++++++------
 net/sched/sch_htb.c       | 20 ++++++++++++--------
 net/sched/sch_ingress.c   | 14 +++++++++++---
 net/sched/sch_multiq.c    |  7 ++++---
 net/sched/sch_prio.c      |  9 +++++----
 net/sched/sch_qfq.c       |  7 ++++---
 net/sched/sch_sfb.c       |  9 +++++----
 net/sched/sch_sfq.c       | 11 ++++++-----
 18 files changed, 141 insertions(+), 92 deletions(-)

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 891d80d..27a1efa 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -109,9 +109,9 @@ static inline void qdisc_run(struct Qdisc *q)
 		__qdisc_run(q);
 }
 
-int tc_classify_compat(struct sk_buff *skb, const struct tcf_proto *tp,
+int tc_classify_compat(struct sk_buff *skb, const struct list_head *head,
 		       struct tcf_result *res);
-int tc_classify(struct sk_buff *skb, const struct tcf_proto *tp,
+int tc_classify(struct sk_buff *skb, const struct list_head *head,
 		struct tcf_result *res);
 
 /* Calculate maximal size of packet seen by hard_start_xmit
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 013d96d..97123cc 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -6,6 +6,7 @@
 #include <linux/rcupdate.h>
 #include <linux/pkt_sched.h>
 #include <linux/pkt_cls.h>
+#include <linux/list.h>
 #include <net/gen_stats.h>
 #include <net/rtnetlink.h>
 
@@ -143,7 +144,7 @@ struct Qdisc_class_ops {
 	void			(*walk)(struct Qdisc *, struct qdisc_walker * arg);
 
 	/* Filter manipulation */
-	struct tcf_proto **	(*tcf_chain)(struct Qdisc *, unsigned long);
+	struct list_head *	(*tcf_chain)(struct Qdisc *, unsigned long);
 	unsigned long		(*bind_tcf)(struct Qdisc *, unsigned long,
 					u32 classid);
 	void			(*unbind_tcf)(struct Qdisc *, unsigned long);
@@ -184,6 +185,8 @@ struct tcf_result {
 	u32		classid;
 };
 
+struct tcf_proto;
+
 struct tcf_proto_ops {
 	struct list_head	head;
 	char			kind[IFNAMSIZ];
@@ -212,7 +215,7 @@ struct tcf_proto_ops {
 
 struct tcf_proto {
 	/* Fast access part */
-	struct tcf_proto	*next;
+	struct list_head	head;
 	void			*root;
 	int			(*classify)(struct sk_buff *,
 					    const struct tcf_proto *,
@@ -376,7 +379,7 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue,
 void __qdisc_calculate_pkt_len(struct sk_buff *skb,
 			       const struct qdisc_size_table *stab);
 void tcf_destroy(struct tcf_proto *tp);
-void tcf_destroy_chain(struct tcf_proto **fl);
+void tcf_destroy_chain(struct list_head *fl);
 
 /* Reset all TX qdiscs greater then index of a device.  */
 static inline void qdisc_reset_all_tx_gt(struct net_device *dev, unsigned int i)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d8c42b1..bcc9987 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -125,8 +125,9 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 	u32 parent;
 	struct net_device *dev;
 	struct Qdisc  *q;
-	struct tcf_proto **back, **chain;
-	struct tcf_proto *tp;
+	struct tcf_proto *back;
+	struct list_head *chain;
+	struct tcf_proto *tp, *res = NULL;
 	const struct tcf_proto_ops *tp_ops;
 	const struct Qdisc_class_ops *cops;
 	unsigned long cl;
@@ -196,21 +197,27 @@ replay:
 		goto errout;
 
 	/* Check the chain for existence of proto-tcf with this priority */
-	for (back = chain; (tp = *back) != NULL; back = &tp->next) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(tp, chain, head) {
+		back = list_next_entry(tp, head);
 		if (tp->prio >= prio) {
 			if (tp->prio == prio) {
 				if (!nprio ||
-				    (tp->protocol != protocol && protocol))
+				    (tp->protocol != protocol && protocol)) {
+					rcu_read_unlock();
 					goto errout;
+				}
+				res = tp;
 			} else
-				tp = NULL;
+				res = NULL;
 			break;
 		}
 	}
+	rcu_read_unlock();
 
 	root_lock = qdisc_root_sleeping_lock(q);
 
-	if (tp == NULL) {
+	if ((tp = res) == NULL) {
 		/* Proto-tcf does not exist, create new one */
 
 		if (tca[TCA_KIND] == NULL || !protocol)
@@ -228,6 +235,7 @@ replay:
 		tp = kzalloc(sizeof(*tp), GFP_KERNEL);
 		if (tp == NULL)
 			goto errout;
+		INIT_LIST_HEAD(&tp->head);
 		err = -ENOENT;
 		tp_ops = tcf_proto_lookup_ops(tca[TCA_KIND]);
 		if (tp_ops == NULL) {
@@ -258,7 +266,7 @@ replay:
 		}
 		tp->ops = tp_ops;
 		tp->protocol = protocol;
-		tp->prio = nprio ? : TC_H_MAJ(tcf_auto_prio(*back));
+		tp->prio = nprio ? : TC_H_MAJ(tcf_auto_prio(back));
 		tp->q = q;
 		tp->classify = tp_ops->classify;
 		tp->classid = parent;
@@ -280,7 +288,7 @@ replay:
 	if (fh == 0) {
 		if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) {
 			spin_lock_bh(root_lock);
-			*back = tp->next;
+			list_del_rcu(&tp->head);
 			spin_unlock_bh(root_lock);
 
 			tfilter_notify(net, skb, n, tp, fh, RTM_DELTFILTER);
@@ -321,8 +329,7 @@ replay:
 	if (err == 0) {
 		if (tp_created) {
 			spin_lock_bh(root_lock);
-			tp->next = *back;
-			*back = tp;
+			list_add_rcu(&tp->head, chain);
 			spin_unlock_bh(root_lock);
 		}
 		tfilter_notify(net, skb, n, tp, fh, RTM_NEWTFILTER);
@@ -417,7 +424,8 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 	int s_t;
 	struct net_device *dev;
 	struct Qdisc *q;
-	struct tcf_proto *tp, **chain;
+	struct tcf_proto *tp;
+	struct list_head *chain;
 	struct tcmsg *tcm = nlmsg_data(cb->nlh);
 	unsigned long cl = 0;
 	const struct Qdisc_class_ops *cops;
@@ -451,7 +459,9 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 
 	s_t = cb->args[0];
 
-	for (tp = *chain, t = 0; tp; tp = tp->next, t++) {
+	t = 0;
+	rcu_read_lock();
+	list_for_each_entry_rcu(tp, chain, head) {
 		if (t < s_t)
 			continue;
 		if (TC_H_MAJ(tcm->tcm_info) &&
@@ -482,7 +492,9 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 		cb->args[1] = arg.w.count + 1;
 		if (arg.w.stop)
 			break;
+		t++;
 	}
+	rcu_read_unlock();
 
 	cb->args[0] = t;
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 1313145..df86686 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -29,6 +29,7 @@
 #include <linux/hrtimer.h>
 #include <linux/lockdep.h>
 #include <linux/slab.h>
+#include <linux/rculist.h>
 
 #include <net/net_namespace.h>
 #include <net/sock.h>
@@ -1772,13 +1773,15 @@ done:
  * to this qdisc, (optionally) tests for protocol and asks
  * specific classifiers.
  */
-int tc_classify_compat(struct sk_buff *skb, const struct tcf_proto *tp,
+int tc_classify_compat(struct sk_buff *skb, const struct list_head *head,
 		       struct tcf_result *res)
 {
+	struct tcf_proto *tp;
 	__be16 protocol = skb->protocol;
-	int err;
+	int err = -1;
 
-	for (; tp; tp = tp->next) {
+	WARN_ON_ONCE(!rcu_read_lock_held());
+	list_for_each_entry_rcu(tp, head, head) {
 		if (tp->protocol != protocol &&
 		    tp->protocol != htons(ETH_P_ALL))
 			continue;
@@ -1789,23 +1792,25 @@ int tc_classify_compat(struct sk_buff *skb, const struct tcf_proto *tp,
 			if (err != TC_ACT_RECLASSIFY && skb->tc_verd)
 				skb->tc_verd = SET_TC_VERD(skb->tc_verd, 0);
 #endif
-			return err;
+			break;
 		}
 	}
-	return -1;
+
+	return err;
 }
 EXPORT_SYMBOL(tc_classify_compat);
 
-int tc_classify(struct sk_buff *skb, const struct tcf_proto *tp,
+int tc_classify(struct sk_buff *skb, const struct list_head *head,
 		struct tcf_result *res)
 {
 	int err = 0;
 #ifdef CONFIG_NET_CLS_ACT
-	const struct tcf_proto *otp = tp;
+	const struct tcf_proto *tp;
+	const struct tcf_proto *otp = list_first_entry(head, struct tcf_proto, head);
 reclassify:
 #endif
 
-	err = tc_classify_compat(skb, tp, res);
+	err = tc_classify_compat(skb, head, res);
 #ifdef CONFIG_NET_CLS_ACT
 	if (err == TC_ACT_RECLASSIFY) {
 		u32 verd = G_TC_VERD(skb->tc_verd);
@@ -1830,16 +1835,20 @@ void tcf_destroy(struct tcf_proto *tp)
 {
 	tp->ops->destroy(tp);
 	module_put(tp->ops->owner);
+	synchronize_rcu();
 	kfree(tp);
 }
 
-void tcf_destroy_chain(struct tcf_proto **fl)
+void tcf_destroy_chain(struct list_head *fl)
 {
-	struct tcf_proto *tp;
+	struct tcf_proto *tp, *n;
+	LIST_HEAD(list);
+	list_splice_init_rcu(fl, &list, synchronize_rcu);
 
-	while ((tp = *fl) != NULL) {
-		*fl = tp->next;
-		tcf_destroy(tp);
+	list_for_each_entry_safe(tp, n, &list, head) {
+		tp->ops->destroy(tp);
+		module_put(tp->ops->owner);
+		kfree(tp);
 	}
 }
 EXPORT_SYMBOL(tcf_destroy_chain);
diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index 1f9c314..20a07c2 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -41,7 +41,7 @@
 
 struct atm_flow_data {
 	struct Qdisc		*q;	/* FIFO, TBF, etc. */
-	struct tcf_proto	*filter_list;
+	struct list_head 	filter_list;
 	struct atm_vcc		*vcc;	/* VCC; NULL if VCC is closed */
 	void			(*old_pop)(struct atm_vcc *vcc,
 					   struct sk_buff *skb); /* chaining */
@@ -273,7 +273,7 @@ static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent,
 		error = -ENOBUFS;
 		goto err_out;
 	}
-	flow->filter_list = NULL;
+	INIT_LIST_HEAD(&flow->filter_list);
 	flow->q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, classid);
 	if (!flow->q)
 		flow->q = &noop_qdisc;
@@ -311,7 +311,7 @@ static int atm_tc_delete(struct Qdisc *sch, unsigned long arg)
 	pr_debug("atm_tc_delete(sch %p,[qdisc %p],flow %p)\n", sch, p, flow);
 	if (list_empty(&flow->list))
 		return -EINVAL;
-	if (flow->filter_list || flow == &p->link)
+	if (!list_empty(&flow->filter_list) || flow == &p->link)
 		return -EBUSY;
 	/*
 	 * Reference count must be 2: one for "keepalive" (set at class
@@ -345,7 +345,7 @@ static void atm_tc_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 	}
 }
 
-static struct tcf_proto **atm_tc_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *atm_tc_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct atm_qdisc_data *p = qdisc_priv(sch);
 	struct atm_flow_data *flow = (struct atm_flow_data *)cl;
@@ -370,9 +370,9 @@ static int atm_tc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	if (TC_H_MAJ(skb->priority) != sch->handle ||
 	    !(flow = (struct atm_flow_data *)atm_tc_get(sch, skb->priority))) {
 		list_for_each_entry(flow, &p->flows, list) {
-			if (flow->filter_list) {
+			if (!list_empty(&flow->filter_list)) {
 				result = tc_classify_compat(skb,
-							    flow->filter_list,
+							    &flow->filter_list,
 							    &res);
 				if (result < 0)
 					continue;
@@ -544,7 +544,7 @@ static int atm_tc_init(struct Qdisc *sch, struct nlattr *opt)
 	if (!p->link.q)
 		p->link.q = &noop_qdisc;
 	pr_debug("atm_tc_init: link (%p) qdisc %p\n", &p->link, p->link.q);
-	p->link.filter_list = NULL;
+	INIT_LIST_HEAD(&p->link.filter_list);
 	p->link.vcc = NULL;
 	p->link.sock = NULL;
 	p->link.classid = sch->handle;
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 2f80d01..a5914ff 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -133,7 +133,7 @@ struct cbq_class {
 	struct gnet_stats_rate_est64 rate_est;
 	struct tc_cbq_xstats	xstats;
 
-	struct tcf_proto	*filter_list;
+	struct list_head	filter_list;
 
 	int			refcnt;
 	int			filters;
@@ -239,8 +239,8 @@ cbq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 		/*
 		 * Step 2+n. Apply classifier.
 		 */
-		if (!head->filter_list ||
-		    (result = tc_classify_compat(skb, head->filter_list, &res)) < 0)
+		if (list_empty(&head->filter_list) ||
+		    (result = tc_classify_compat(skb, &head->filter_list, &res)) < 0)
 			goto fallback;
 
 		cl = (void *)res.class;
@@ -1881,6 +1881,7 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
 		}
 	}
 
+	INIT_LIST_HEAD(&cl->filter_list);
 	cl->R_tab = rtab;
 	rtab = NULL;
 	cl->refcnt = 1;
@@ -1976,7 +1977,7 @@ static int cbq_delete(struct Qdisc *sch, unsigned long arg)
 	return 0;
 }
 
-static struct tcf_proto **cbq_find_tcf(struct Qdisc *sch, unsigned long arg)
+static struct list_head *cbq_find_tcf(struct Qdisc *sch, unsigned long arg)
 {
 	struct cbq_sched_data *q = qdisc_priv(sch);
 	struct cbq_class *cl = (struct cbq_class *)arg;
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index ddd73cb..9b36830 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -58,7 +58,7 @@ struct choke_sched_data {
 
 /* Variables */
 	struct red_vars  vars;
-	struct tcf_proto *filter_list;
+	struct list_head filter_list;
 	struct {
 		u32	prob_drop;	/* Early probability drops */
 		u32	prob_mark;	/* Early probability marks */
@@ -202,7 +202,7 @@ static bool choke_classify(struct sk_buff *skb,
 	struct tcf_result res;
 	int result;
 
-	result = tc_classify(skb, q->filter_list, &res);
+	result = tc_classify(skb, &q->filter_list, &res);
 	if (result >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
@@ -256,7 +256,7 @@ static bool choke_match_random(const struct choke_sched_data *q,
 		return false;
 
 	oskb = choke_peek_random(q, pidx);
-	if (q->filter_list)
+	if (!list_empty(&q->filter_list))
 		return choke_get_classid(nskb) == choke_get_classid(oskb);
 
 	return choke_match_flow(oskb, nskb);
@@ -268,7 +268,7 @@ static int choke_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	const struct red_parms *p = &q->parms;
 	int ret = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 
-	if (q->filter_list) {
+	if (!list_empty(&q->filter_list)) {
 		/* If using external classifiers, get result and record it. */
 		if (!choke_classify(skb, sch, &ret))
 			goto other_drop;	/* Packet was eaten by filter */
@@ -476,6 +476,7 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt)
 
 	q->flags = ctl->flags;
 	q->limit = ctl->limit;
+	INIT_LIST_HEAD(&q->filter_list);
 
 	red_set_parms(&q->parms, ctl->qth_min, ctl->qth_max, ctl->Wlog,
 		      ctl->Plog, ctl->Scell_log,
@@ -566,7 +567,7 @@ static unsigned long choke_bind(struct Qdisc *sch, unsigned long parent,
 	return 0;
 }
 
-static struct tcf_proto **choke_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *choke_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct choke_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 8302717..62f45ac 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -35,7 +35,7 @@ struct drr_class {
 
 struct drr_sched {
 	struct list_head		active;
-	struct tcf_proto		*filter_list;
+	struct list_head		filter_list;
 	struct Qdisc_class_hash		clhash;
 };
 
@@ -184,7 +184,7 @@ static void drr_put_class(struct Qdisc *sch, unsigned long arg)
 		drr_destroy_class(sch, cl);
 }
 
-static struct tcf_proto **drr_tcf_chain(struct Qdisc *sch, unsigned long cl)
+static struct list_head *drr_tcf_chain(struct Qdisc *sch, unsigned long cl)
 {
 	struct drr_sched *q = qdisc_priv(sch);
 
@@ -328,7 +328,7 @@ static struct drr_class *drr_classify(struct sk_buff *skb, struct Qdisc *sch,
 	}
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
-	result = tc_classify(skb, q->filter_list, &res);
+	result = tc_classify(skb, &q->filter_list, &res);
 	if (result >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
@@ -443,6 +443,7 @@ static int drr_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
 	if (err < 0)
 		return err;
 	INIT_LIST_HEAD(&q->active);
+	INIT_LIST_HEAD(&q->filter_list);
 	return 0;
 }
 
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 49d6ef3..4489ffe 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -37,7 +37,7 @@
 
 struct dsmark_qdisc_data {
 	struct Qdisc		*q;
-	struct tcf_proto	*filter_list;
+	struct list_head	filter_list;
 	u8			*mask;	/* "owns" the array */
 	u8			*value;
 	u16			indices;
@@ -186,7 +186,7 @@ ignore:
 	}
 }
 
-static inline struct tcf_proto **dsmark_find_tcf(struct Qdisc *sch,
+static inline struct list_head *dsmark_find_tcf(struct Qdisc *sch,
 						 unsigned long cl)
 {
 	struct dsmark_qdisc_data *p = qdisc_priv(sch);
@@ -229,7 +229,7 @@ static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		skb->tc_index = TC_H_MIN(skb->priority);
 	else {
 		struct tcf_result res;
-		int result = tc_classify(skb, p->filter_list, &res);
+		int result = tc_classify(skb, &p->filter_list, &res);
 
 		pr_debug("result %d class 0x%04x\n", result, res.classid);
 
@@ -380,6 +380,7 @@ static int dsmark_init(struct Qdisc *sch, struct nlattr *opt)
 	p->indices = indices;
 	p->default_index = default_index;
 	p->set_tc_index = nla_get_flag(tb[TCA_DSMARK_SET_TC_INDEX]);
+	INIT_LIST_HEAD(&p->filter_list);
 
 	p->q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, sch->handle);
 	if (p->q == NULL)
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 5578628..3c19917 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -52,7 +52,7 @@ struct fq_codel_flow {
 }; /* please try to keep this structure <= 64 bytes */
 
 struct fq_codel_sched_data {
-	struct tcf_proto *filter_list;	/* optional external classifier */
+	struct list_head filter_list;	/* optional external classifier */
 	struct fq_codel_flow *flows;	/* Flows table [flows_cnt] */
 	u32		*backlogs;	/* backlog table [flows_cnt] */
 	u32		flows_cnt;	/* number of flows */
@@ -92,11 +92,11 @@ static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
 	    TC_H_MIN(skb->priority) <= q->flows_cnt)
 		return TC_H_MIN(skb->priority);
 
-	if (!q->filter_list)
+	if (list_emty(&q->filter_list))
 		return fq_codel_hash(q, skb) + 1;
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
-	result = tc_classify(skb, q->filter_list, &res);
+	result = tc_classify(skb, &q->filter_list, &res);
 	if (result >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
@@ -393,6 +393,7 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt)
 	q->perturbation = net_random();
 	INIT_LIST_HEAD(&q->new_flows);
 	INIT_LIST_HEAD(&q->old_flows);
+	INIT_LIST_HEAD(&q->filter_list);
 	codel_params_init(&q->cparams);
 	codel_stats_init(&q->cstats);
 	q->cparams.ecn = true;
@@ -501,7 +502,7 @@ static void fq_codel_put(struct Qdisc *q, unsigned long cl)
 {
 }
 
-static struct tcf_proto **fq_codel_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *fq_codel_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct fq_codel_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index c407561..8bb4ade 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -116,7 +116,7 @@ struct hfsc_class {
 	struct gnet_stats_queue qstats;
 	struct gnet_stats_rate_est64 rate_est;
 	unsigned int	level;		/* class level in hierarchy */
-	struct tcf_proto *filter_list;	/* filter list */
+	struct list_head filter_list;	/* filter list */
 	unsigned int	filter_cnt;	/* filter count */
 
 	struct hfsc_sched *sched;	/* scheduler data */
@@ -1083,6 +1083,7 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	cl->refcnt    = 1;
 	cl->sched     = q;
 	cl->cl_parent = parent;
+	INIT_LIST_HEAD(&cl->filter_list);
 	cl->qdisc = qdisc_create_dflt(sch->dev_queue,
 				      &pfifo_qdisc_ops, classid);
 	if (cl->qdisc == NULL)
@@ -1151,7 +1152,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 	struct hfsc_sched *q = qdisc_priv(sch);
 	struct hfsc_class *head, *cl;
 	struct tcf_result res;
-	struct tcf_proto *tcf;
+	struct list_head *list;
 	int result;
 
 	if (TC_H_MAJ(skb->priority ^ sch->handle) == 0 &&
@@ -1161,8 +1162,10 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 	head = &q->root;
-	tcf = q->root.filter_list;
-	while (tcf && (result = tc_classify(skb, tcf, &res)) >= 0) {
+	list = &q->root.filter_list;
+	while (list) {
+		if ((result = tc_classify(skb, list, &res)) < 0)
+			break;
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
 		case TC_ACT_QUEUED:
@@ -1185,7 +1188,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 			return cl; /* hit leaf class */
 
 		/* apply inner filter chain */
-		tcf = cl->filter_list;
+		list = &cl->filter_list;
 		head = cl;
 	}
 
@@ -1285,7 +1288,7 @@ hfsc_unbind_tcf(struct Qdisc *sch, unsigned long arg)
 	cl->filter_cnt--;
 }
 
-static struct tcf_proto **
+static struct list_head *
 hfsc_tcf_chain(struct Qdisc *sch, unsigned long arg)
 {
 	struct hfsc_sched *q = qdisc_priv(sch);
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 0db5a6e..c28e405 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -103,7 +103,7 @@ struct htb_class {
 	u32			prio;		/* these two are used only by leaves... */
 	int			quantum;	/* but stored for parent-to-leaf return */
 
-	struct tcf_proto	*filter_list;	/* class attached filters */
+	struct list_head	filter_list;	/* class attached filters */
 	int			filter_cnt;
 	int			refcnt;		/* usage count of this class */
 
@@ -153,7 +153,7 @@ struct htb_sched {
 	int			rate2quantum;	/* quant = rate / rate2quantum */
 
 	/* filters for qdisc itself */
-	struct tcf_proto	*filter_list;
+	struct list_head	filter_list;
 
 #define HTB_WARN_TOOMANYEVENTS	0x1
 	unsigned int		warned;	/* only one warning */
@@ -209,7 +209,7 @@ static struct htb_class *htb_classify(struct sk_buff *skb, struct Qdisc *sch,
 	struct htb_sched *q = qdisc_priv(sch);
 	struct htb_class *cl;
 	struct tcf_result res;
-	struct tcf_proto *tcf;
+	struct list_head *head;
 	int result;
 
 	/* allow to select class by setting skb->priority to valid classid;
@@ -223,8 +223,10 @@ static struct htb_class *htb_classify(struct sk_buff *skb, struct Qdisc *sch,
 		return cl;
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
-	tcf = q->filter_list;
-	while (tcf && (result = tc_classify(skb, tcf, &res)) >= 0) {
+	head = &q->filter_list;
+	while (!list_empty(head)) {
+		if ((result = tc_classify(skb, head, &res)) < 0)
+			break;
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
 		case TC_ACT_QUEUED:
@@ -246,7 +248,7 @@ static struct htb_class *htb_classify(struct sk_buff *skb, struct Qdisc *sch,
 			return cl;	/* we hit leaf; return it */
 
 		/* we have got inner class; apply inner filter chain */
-		tcf = cl->filter_list;
+		head = &cl->filter_list;
 	}
 	/* classification failed; try to use default class */
 	cl = htb_find(TC_H_MAKE(TC_H_MAJ(sch->handle), q->defcls), sch);
@@ -1040,6 +1042,7 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt)
 	qdisc_watchdog_init(&q->watchdog, sch);
 	INIT_WORK(&q->work, htb_work_func);
 	skb_queue_head_init(&q->direct_queue);
+	INIT_LIST_HEAD(&q->filter_list);
 
 	if (tb[TCA_HTB_DIRECT_QLEN])
 		q->direct_qlen = nla_get_u32(tb[TCA_HTB_DIRECT_QLEN]);
@@ -1413,6 +1416,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 		cl->refcnt = 1;
 		cl->children = 0;
 		INIT_LIST_HEAD(&cl->un.leaf.drop_list);
+		INIT_LIST_HEAD(&cl->filter_list);
 		RB_CLEAR_NODE(&cl->pq_node);
 
 		for (prio = 0; prio < TC_HTB_NUMPRIO; prio++)
@@ -1518,11 +1522,11 @@ failure:
 	return err;
 }
 
-static struct tcf_proto **htb_find_tcf(struct Qdisc *sch, unsigned long arg)
+static struct list_head *htb_find_tcf(struct Qdisc *sch, unsigned long arg)
 {
 	struct htb_sched *q = qdisc_priv(sch);
 	struct htb_class *cl = (struct htb_class *)arg;
-	struct tcf_proto **fl = cl ? &cl->filter_list : &q->filter_list;
+	struct list_head *fl = cl ? &cl->filter_list : &q->filter_list;
 
 	return fl;
 }
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index bce1665..b182e0c 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -17,7 +17,7 @@
 
 
 struct ingress_qdisc_data {
-	struct tcf_proto	*filter_list;
+	struct list_head	filter_list;
 };
 
 /* ------------------------- Class/flow operations ------------------------- */
@@ -46,7 +46,7 @@ static void ingress_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 {
 }
 
-static struct tcf_proto **ingress_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *ingress_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct ingress_qdisc_data *p = qdisc_priv(sch);
 
@@ -61,7 +61,7 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	struct tcf_result res;
 	int result;
 
-	result = tc_classify(skb, p->filter_list, &res);
+	result = tc_classify(skb, &p->filter_list, &res);
 
 	qdisc_bstats_update(sch, skb);
 	switch (result) {
@@ -108,6 +108,13 @@ nla_put_failure:
 	return -1;
 }
 
+static int ingress_init(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct ingress_qdisc_data *q = qdisc_priv(sch);
+	INIT_LIST_HEAD(&q->filter_list);
+	return 0;
+}
+
 static const struct Qdisc_class_ops ingress_class_ops = {
 	.leaf		=	ingress_leaf,
 	.get		=	ingress_get,
@@ -119,6 +126,7 @@ static const struct Qdisc_class_ops ingress_class_ops = {
 };
 
 static struct Qdisc_ops ingress_qdisc_ops __read_mostly = {
+	.init		=	ingress_init,
 	.cl_ops		=	&ingress_class_ops,
 	.id		=	"ingress",
 	.priv_size	=	sizeof(struct ingress_qdisc_data),
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index afb050a..e4be093 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -31,7 +31,7 @@ struct multiq_sched_data {
 	u16 bands;
 	u16 max_bands;
 	u16 curband;
-	struct tcf_proto *filter_list;
+	struct list_head filter_list;
 	struct Qdisc **queues;
 };
 
@@ -45,7 +45,7 @@ multiq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 	int err;
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
-	err = tc_classify(skb, q->filter_list, &res);
+	err = tc_classify(skb, &q->filter_list, &res);
 #ifdef CONFIG_NET_CLS_ACT
 	switch (err) {
 	case TC_ACT_STOLEN:
@@ -258,6 +258,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt)
 	if (opt == NULL)
 		return -EINVAL;
 
+	INIT_LIST_HEAD(&q->filter_list);
 	q->max_bands = qdisc_dev(sch)->num_tx_queues;
 
 	q->queues = kcalloc(q->max_bands, sizeof(struct Qdisc *), GFP_KERNEL);
@@ -388,7 +389,7 @@ static void multiq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
 	}
 }
 
-static struct tcf_proto **multiq_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *multiq_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct multiq_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 79359b6..f4ee09f 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -24,7 +24,7 @@
 
 struct prio_sched_data {
 	int bands;
-	struct tcf_proto *filter_list;
+	struct list_head filter_list;
 	u8  prio2band[TC_PRIO_MAX+1];
 	struct Qdisc *queues[TCQ_PRIO_BANDS];
 };
@@ -40,7 +40,7 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 	if (TC_H_MAJ(skb->priority) != sch->handle) {
-		err = tc_classify(skb, q->filter_list, &res);
+		err = tc_classify(skb, &q->filter_list, &res);
 #ifdef CONFIG_NET_CLS_ACT
 		switch (err) {
 		case TC_ACT_STOLEN:
@@ -50,7 +50,7 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 			return NULL;
 		}
 #endif
-		if (!q->filter_list || err < 0) {
+		if (list_empty(&q->filter_list) || err < 0) {
 			if (TC_H_MAJ(band))
 				band = 0;
 			return q->queues[q->prio2band[band & TC_PRIO_MAX]];
@@ -235,6 +235,7 @@ static int prio_init(struct Qdisc *sch, struct nlattr *opt)
 		if ((err = prio_tune(sch, opt)) != 0)
 			return err;
 	}
+	INIT_LIST_HEAD(&q->filter_list);
 	return 0;
 }
 
@@ -351,7 +352,7 @@ static void prio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
 	}
 }
 
-static struct tcf_proto **prio_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *prio_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct prio_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 8056fb4..c43f85a 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -181,7 +181,7 @@ struct qfq_group {
 };
 
 struct qfq_sched {
-	struct tcf_proto *filter_list;
+	struct list_head	filter_list;
 	struct Qdisc_class_hash clhash;
 
 	u64			oldV, V;	/* Precise virtual times. */
@@ -576,7 +576,7 @@ static void qfq_put_class(struct Qdisc *sch, unsigned long arg)
 		qfq_destroy_class(sch, cl);
 }
 
-static struct tcf_proto **qfq_tcf_chain(struct Qdisc *sch, unsigned long cl)
+static struct list_head *qfq_tcf_chain(struct Qdisc *sch, unsigned long cl)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
 
@@ -714,7 +714,7 @@ static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch,
 	}
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
-	result = tc_classify(skb, q->filter_list, &res);
+	result = tc_classify(skb, &q->filter_list, &res);
 	if (result >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
@@ -1498,6 +1498,7 @@ static int qfq_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
 	}
 
 	INIT_HLIST_HEAD(&q->nonfull_aggs);
+	INIT_LIST_HEAD(&q->filter_list);
 
 	return 0;
 }
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index 30ea467..20fcc6d 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -55,7 +55,7 @@ struct sfb_bins {
 
 struct sfb_sched_data {
 	struct Qdisc	*qdisc;
-	struct tcf_proto *filter_list;
+	struct list_head filter_list;
 	unsigned long	rehash_interval;
 	unsigned long	warmup_time;	/* double buffering warmup time in jiffies */
 	u32		max;
@@ -259,7 +259,7 @@ static bool sfb_classify(struct sk_buff *skb, struct sfb_sched_data *q,
 	struct tcf_result res;
 	int result;
 
-	result = tc_classify(skb, q->filter_list, &res);
+	result = tc_classify(skb, &q->filter_list, &res);
 	if (result >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
@@ -306,7 +306,7 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		}
 	}
 
-	if (q->filter_list) {
+	if (!list_empty(&q->filter_list)) {
 		/* If using external classifiers, get result and record it. */
 		if (!sfb_classify(skb, q, &ret, &salt))
 			goto other_drop;
@@ -533,6 +533,7 @@ static int sfb_change(struct Qdisc *sch, struct nlattr *opt)
 	q->tokens_avail = ctl->penalty_burst;
 	q->token_time = jiffies;
 
+	INIT_LIST_HEAD(&q->filter_list);
 	q->slot = 0;
 	q->double_buffering = false;
 	sfb_zero_all_buckets(q);
@@ -660,7 +661,7 @@ static void sfb_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 	}
 }
 
-static struct tcf_proto **sfb_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *sfb_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct sfb_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 76f01e0..2a5b2a4 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -125,7 +125,7 @@ struct sfq_sched_data {
 	u8		cur_depth;	/* depth of longest slot */
 	u8		flags;
 	unsigned short  scaled_quantum; /* SFQ_ALLOT_SIZE(quantum) */
-	struct tcf_proto *filter_list;
+	struct list_head filter_list;
 	sfq_index	*ht;		/* Hash table ('divisor' slots) */
 	struct sfq_slot	*slots;		/* Flows table ('maxflows' entries) */
 
@@ -194,13 +194,13 @@ static unsigned int sfq_classify(struct sk_buff *skb, struct Qdisc *sch,
 	    TC_H_MIN(skb->priority) <= q->divisor)
 		return TC_H_MIN(skb->priority);
 
-	if (!q->filter_list) {
+	if (list_empty(&q->filter_list)) {
 		skb_flow_dissect(skb, &sfq_skb_cb(skb)->keys);
 		return sfq_hash(q, skb) + 1;
 	}
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
-	result = tc_classify(skb, q->filter_list, &res);
+	result = tc_classify(skb, &q->filter_list, &res);
 	if (result >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
 		switch (result) {
@@ -630,7 +630,7 @@ static void sfq_perturbation(unsigned long arg)
 
 	spin_lock(root_lock);
 	q->perturbation = net_random();
-	if (!q->filter_list && q->tail)
+	if (list_empty(&q->filter_list) && q->tail)
 		sfq_rehash(sch);
 	spin_unlock(root_lock);
 
@@ -760,6 +760,7 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
 	q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
 	q->perturb_period = 0;
 	q->perturbation = net_random();
+	INIT_LIST_HEAD(&q->filter_list);
 
 	if (opt) {
 		int err = sfq_change(sch, opt);
@@ -846,7 +847,7 @@ static void sfq_put(struct Qdisc *q, unsigned long cl)
 {
 }
 
-static struct tcf_proto **sfq_find_tcf(struct Qdisc *sch, unsigned long cl)
+static struct list_head *sfq_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct sfq_sched_data *q = qdisc_priv(sch);
 
-- 
1.8.3.1

^ permalink raw reply related

* [RFC Patch net-next 0/4] net_sched: make ingress filters lockless
From: Cong Wang @ 2014-01-09 18:19 UTC (permalink / raw)
  To: netdev
  Cc: Cong Wang, John Fastabend, Eric Dumazet, David S. Miller,
	Jamal Hadi Salim

This patch tries to switch filter list to using struct
list_head, so that on the read side, the list can be traversed
with RCU read lock. Same for actions.

I hope either on egress or ingress classify is already protected by RCU
read lock, but I don't pretend I fully understanding qdisc locking.

Also, I am not sure I use RCU API's correctly at all. At least
I don't see any warning with CONFIG_PROVE_RCU=y.

Without this patch, the spin_lock easily appears on the top of
my perf top with 4 netperf sessions (4 is the number of CPU)
and with 1000 u32 filters on ingress.

Question 1:
Am I still missing something on qdisc locking?

Question 2:
Is it okay to call qdisc_bstats_update() without any lock?

Please comment.

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Cong Wang (4):
  net_sched: switch filter list to list_head
  net_sched: avoid holding qdisc lock for filters
  net_sched: use RCU for tc actions traverse
  net_sched: make ingress qdisc lockless

 include/net/pkt_cls.h     | 25 ++++--------------------
 include/net/pkt_sched.h   |  4 ++--
 include/net/sch_generic.h | 14 +++++++++-----
 net/core/dev.c            |  2 --
 net/sched/act_api.c       | 12 +++++++-----
 net/sched/cls_api.c       | 48 ++++++++++++++++++++++++++++-------------------
 net/sched/sch_api.c       | 35 +++++++++++++++++++++-------------
 net/sched/sch_atm.c       | 14 +++++++-------
 net/sched/sch_cbq.c       |  9 +++++----
 net/sched/sch_choke.c     | 11 ++++++-----
 net/sched/sch_drr.c       |  7 ++++---
 net/sched/sch_dsmark.c    |  7 ++++---
 net/sched/sch_fq_codel.c  |  9 +++++----
 net/sched/sch_hfsc.c      | 15 +++++++++------
 net/sched/sch_htb.c       | 20 ++++++++++++--------
 net/sched/sch_ingress.c   | 14 +++++++++++---
 net/sched/sch_multiq.c    |  7 ++++---
 net/sched/sch_prio.c      |  9 +++++----
 net/sched/sch_qfq.c       |  7 ++++---
 net/sched/sch_sfb.c       |  9 +++++----
 net/sched/sch_sfq.c       | 11 ++++++-----
 21 files changed, 160 insertions(+), 129 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* Re: [net-next 2/7] ixgbe: Indicate removal state explicitly
From: Rustad, Mark D @ 2014-01-09 18:18 UTC (permalink / raw)
  To: Scott Feldman
  Cc: Brown, Aaron F, David Miller, Netdev, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <2537C4E1-5434-4C6B-95A1-2360BA0803FB@intel.com>

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

On Jan 9, 2014, at 9:27 AM, Rustad, Mark D <mark.d.rustad@intel.com> wrote:

> On Jan 8, 2014, at 12:35 AM, Scott Feldman <sfeldma@gmail.com> wrote:
> 
>> On Jan 7, 2014, at 11:40 PM, Aaron Brown <aaron.f.brown@intel.com> wrote:
>> 
>>> From: Mark Rustad <mark.d.rustad@intel.com>
>>> 
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
>>> index 5e157ac..480c5c1 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
>>> @@ -124,6 +124,11 @@ s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw);
>>> s32 ixgbe_get_thermal_sensor_data_generic(struct ixgbe_hw *hw);
>>> s32 ixgbe_init_thermal_sensor_thresh_generic(struct ixgbe_hw *hw);
>>> 
>>> +#define IXGBE_FAILED_READ_REG 0xffffffffU
>>> +
>>> +void ixgbe_check_remove(struct ixgbe_hw *hw, u32 reg);
>>> +#define IXGBE_REMOVED(a) unlikely(!(a))
>> 
>> IXGBE_REMOVED seems pretty closely tied to hw->hw_addr, but the macro turns any input into !input.  Maybe an inline that takes a *hw?
> 
> An earlier version had two different inputs, but not any more. I will change it to take a hw parameter. Good suggestion.

Actually, I responded too quickly. Although my comment on there previously having been two different inputs was true, that isn't the real reason for this form. The reason is that the check needs to be done on the value retrieved by ACCESS_ONCE - the address that will be used for the access. If the parameter were hw, then a separate dereference could occur, which is definitely not wanted.

One could argue that the macro should just be eliminated and the expansion put everywhere, but I feel that the code is much clearer with the macro. Unless there is another suggestion, I think this should stay as it is in this series.

-- 
Mark Rustad, Networking Division, Intel Corporation


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

^ permalink raw reply

* [PATCH net 2/2] qlcnic: Fix ethtool statistics length calculation
From: Shahed Shaikh @ 2014-01-09 17:41 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Shahed Shaikh
In-Reply-To: <1389289265-9586-1-git-send-email-shahed.shaikh@qlogic.com>

From: Shahed Shaikh <shahed.shaikh@qlogic.com>

o Consider number of Tx queues while calculating the length of
  Tx statistics as part of ethtool stats.
o Calculate statistics lenght properly for 82xx and 83xx adapter

Signed-off-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
---
 .../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c    | 39 ++++++++++++----------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 7b36340..6b08194 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -167,27 +167,35 @@ static const char qlcnic_gstrings_test[][ETH_GSTRING_LEN] = {
 
 #define QLCNIC_TEST_LEN	ARRAY_SIZE(qlcnic_gstrings_test)
 
-static inline int qlcnic_82xx_statistics(void)
+static inline int qlcnic_82xx_statistics(struct qlcnic_adapter *adapter)
 {
-	return ARRAY_SIZE(qlcnic_device_gstrings_stats) +
-	       ARRAY_SIZE(qlcnic_83xx_mac_stats_strings);
+	return ARRAY_SIZE(qlcnic_gstrings_stats) +
+	       ARRAY_SIZE(qlcnic_83xx_mac_stats_strings) +
+	       QLCNIC_TX_STATS_LEN * adapter->drv_tx_rings;
 }
 
-static inline int qlcnic_83xx_statistics(void)
+static inline int qlcnic_83xx_statistics(struct qlcnic_adapter *adapter)
 {
-	return ARRAY_SIZE(qlcnic_83xx_tx_stats_strings) +
+	return ARRAY_SIZE(qlcnic_gstrings_stats) +
+	       ARRAY_SIZE(qlcnic_83xx_tx_stats_strings) +
 	       ARRAY_SIZE(qlcnic_83xx_mac_stats_strings) +
-	       ARRAY_SIZE(qlcnic_83xx_rx_stats_strings);
+	       ARRAY_SIZE(qlcnic_83xx_rx_stats_strings) +
+	       QLCNIC_TX_STATS_LEN * adapter->drv_tx_rings;
 }
 
 static int qlcnic_dev_statistics_len(struct qlcnic_adapter *adapter)
 {
-	if (qlcnic_82xx_check(adapter))
-		return qlcnic_82xx_statistics();
-	else if (qlcnic_83xx_check(adapter))
-		return qlcnic_83xx_statistics();
-	else
-		return -1;
+	int len = -1;
+
+	if (qlcnic_82xx_check(adapter)) {
+		len = qlcnic_82xx_statistics(adapter);
+		if (adapter->flags & QLCNIC_ESWITCH_ENABLED)
+			len += ARRAY_SIZE(qlcnic_device_gstrings_stats);
+	} else if (qlcnic_83xx_check(adapter)) {
+		len = qlcnic_83xx_statistics(adapter);
+	}
+
+	return len;
 }
 
 #define	QLCNIC_TX_INTR_NOT_CONFIGURED	0X78563412
@@ -920,18 +928,13 @@ static int qlcnic_eeprom_test(struct net_device *dev)
 
 static int qlcnic_get_sset_count(struct net_device *dev, int sset)
 {
-	int len;
 
 	struct qlcnic_adapter *adapter = netdev_priv(dev);
 	switch (sset) {
 	case ETH_SS_TEST:
 		return QLCNIC_TEST_LEN;
 	case ETH_SS_STATS:
-		len = qlcnic_dev_statistics_len(adapter) + QLCNIC_STATS_LEN;
-		if ((adapter->flags & QLCNIC_ESWITCH_ENABLED) ||
-		    qlcnic_83xx_check(adapter))
-			return len;
-		return qlcnic_82xx_statistics();
+		return qlcnic_dev_statistics_len(adapter);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net 1/2] qlcnic: Fix bug in TX statistics
From: Shahed Shaikh @ 2014-01-09 17:41 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Manish Chopra
In-Reply-To: <1389289265-9586-1-git-send-email-shahed.shaikh@qlogic.com>

From: Manish Chopra <manish.chopra@qlogic.com>

o Driver was not updating TX stats so it was not populating
  statistics in `ifconfig` command output.

Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h         | 1 +
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c | 2 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c    | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index ff80cd8..f2a7c71 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -1711,6 +1711,7 @@ int qlcnic_83xx_init_mailbox_work(struct qlcnic_adapter *);
 void qlcnic_83xx_detach_mailbox_work(struct qlcnic_adapter *);
 void qlcnic_83xx_reinit_mbx_work(struct qlcnic_mailbox *mbx);
 void qlcnic_83xx_free_mailbox(struct qlcnic_mailbox *mbx);
+void qlcnic_update_stats(struct qlcnic_adapter *);
 
 /* Adapter hardware abstraction */
 struct qlcnic_hardware_ops {
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index e3be276..7b36340 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -1267,7 +1267,7 @@ static u64 *qlcnic_fill_stats(u64 *data, void *stats, int type)
 	return data;
 }
 
-static void qlcnic_update_stats(struct qlcnic_adapter *adapter)
+void qlcnic_update_stats(struct qlcnic_adapter *adapter)
 {
 	struct qlcnic_host_tx_ring *tx_ring;
 	int ring;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index b8a245a..550791b 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -2780,6 +2780,9 @@ static struct net_device_stats *qlcnic_get_stats(struct net_device *netdev)
 	struct qlcnic_adapter *adapter = netdev_priv(netdev);
 	struct net_device_stats *stats = &netdev->stats;
 
+	if (test_bit(__QLCNIC_DEV_UP, &adapter->state))
+		qlcnic_update_stats(adapter);
+
 	stats->rx_packets = adapter->stats.rx_pkts + adapter->stats.lro_pkts;
 	stats->tx_packets = adapter->stats.xmitfinished;
 	stats->rx_bytes = adapter->stats.rxbytes + adapter->stats.lrobytes;
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net 0/2] qlcnic: Bug fixes
From: Shahed Shaikh @ 2014-01-09 17:41 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Shahed Shaikh

From: Shahed Shaikh <shahed.shaikh@qlogic.com>

Hi David,

This patch series has couple of bug fixes related to statistics.

Please apply this series to net.

Thanks,
Shahed

Manish Chopra (1):
  qlcnic: Fix bug in TX statistics

Shahed Shaikh (1):
  qlcnic: Fix ethtool statistics length calculation

 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h        |  1 +
 .../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c    | 41 ++++++++++++----------
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c   |  3 ++
 3 files changed, 26 insertions(+), 19 deletions(-)

-- 
1.8.1.4

^ permalink raw reply

* Re: [Xen-devel] [PATCH net-next v3 1/9] xen-netback: Introduce TX grant map definitions
From: Stefano Stabellini @ 2014-01-09 18:09 UTC (permalink / raw)
  To: David Vrabel
  Cc: Stefano Stabellini, Wei Liu, jonathan.davies, Ian Campbell,
	netdev, linux-kernel, Zoltan Kiss, xen-devel,
	Roger Pau Monné, Konrad Rzeszutek Wilk
In-Reply-To: <52CEE084.5000409@citrix.com>

On Thu, 9 Jan 2014, David Vrabel wrote:
> On 09/01/14 17:28, Stefano Stabellini wrote:
> > On Thu, 9 Jan 2014, David Vrabel wrote:
> >> On 09/01/14 15:30, Wei Liu wrote:
> >>> On Wed, Jan 08, 2014 at 12:10:10AM +0000, Zoltan Kiss wrote:
> >>>> This patch contains the new definitions necessary for grant mapping.
> >>>>
> >>>> v2:
> >>>> - move unmapping to separate thread. The NAPI instance has to be scheduled
> >>>>   even from thread context, which can cause huge delays
> >>>> - that causes unfortunately bigger struct xenvif
> >>>> - store grant handle after checking validity
> >>>>
> >>>> v3:
> >>>> - fix comment in xenvif_tx_dealloc_action()
> >>>> - call unmap hypercall directly instead of gnttab_unmap_refs(), which does
> >>>>   unnecessary m2p_override. Also remove pages_to_[un]map members
> >>>
> >>> Is it worthy to have another function call
> >>> gnttab_unmap_refs_no_m2p_override in Xen core driver, or just add a
> >>> parameter to control wether we need to touch m2p_override? I *think* it
> >>> will benefit block driver as well?
> >>
> >> add_m2p_override and remove_m2p_override calls should be moved into the
> >> gntdev device as that should be the only user.
> > 
> > First of all the gntdev device is common code, while the m2p_override is
> > an x86 concept.
> 
> m2p_add_override() and m2p_remove_override() are already called from
> common code and ARM already provides inline stubs.

This is the right time to fix it, then :)
Maybe we should add the m2p_add_override call to the x86 implementation
of set_phys_to_machine, or maybe we need a new generic
set_machine_to_phys call.


> The m2p override mechanism is also broken by design (local PFN to
> foreign MFN may be many-to-one, but the m2p override only works if local
> PFN to foreign MFN is one-to-one). So I want the m2p override to be only
> used where it is /currently/ necessary.  I think there should be no new
> users of it nor should it be considered a fix for any other use case.

I agree, but I think that we have different views on the use case.
To me the m2p_override use case is "everywhere an mfn_to_pfn translation
is required", that unfortunately is potentially everywhere at this time.

I would love to restrict it further but at the very least we would need
something written down under Documentation. Otherwise when the next
Linux hacker comes along with a performance optimization for her new
network driver that breaks Xen because Xen is incapable of doing mfn to
pfn translations, the maintainers might (rightfully) decide that it is
simply our problem.

^ permalink raw reply

* Re: [PATCH net] bnx2x: fix DMA unmapping of TSO split BDs
From: Eric Dumazet @ 2014-01-09 17:52 UTC (permalink / raw)
  To: Michal Schmidt
  Cc: netdev, Ariel Elior, Yuval Mintz, Dmitry Kravkov, David Miller
In-Reply-To: <1389274587-6097-1-git-send-email-mschmidt@redhat.com>

On Thu, 2014-01-09 at 14:36 +0100, Michal Schmidt wrote:
> bnx2x triggers warnings with CONFIG_DMA_API_DEBUG=y:
> 
>   WARNING: CPU: 0 PID: 2253 at lib/dma-debug.c:887 check_unmap+0xf8/0x920()
>   bnx2x 0000:28:00.0: DMA-API: device driver frees DMA memory with
>   different size [device address=0x00000000da2b389e] [map size=1490 bytes]
>   [unmap size=66 bytes]
> 
> The reason is that bnx2x splits a TSO BD into two BDs (headers + data)
> using one DMA mapping for both, but it uses only the length of the first
> BD when unmapping.
> 
> This patch fixes the bug by unmapping the whole length of the two BDs.
> 
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [Xen-devel] [PATCH net-next v3 1/9] xen-netback: Introduce TX grant map definitions
From: David Vrabel @ 2014-01-09 17:46 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Wei Liu, jonathan.davies, Ian Campbell, netdev, linux-kernel,
	Zoltan Kiss, xen-devel, Roger Pau Monné,
	Konrad Rzeszutek Wilk
In-Reply-To: <alpine.DEB.2.02.1401091722150.21510@kaball.uk.xensource.com>

On 09/01/14 17:28, Stefano Stabellini wrote:
> On Thu, 9 Jan 2014, David Vrabel wrote:
>> On 09/01/14 15:30, Wei Liu wrote:
>>> On Wed, Jan 08, 2014 at 12:10:10AM +0000, Zoltan Kiss wrote:
>>>> This patch contains the new definitions necessary for grant mapping.
>>>>
>>>> v2:
>>>> - move unmapping to separate thread. The NAPI instance has to be scheduled
>>>>   even from thread context, which can cause huge delays
>>>> - that causes unfortunately bigger struct xenvif
>>>> - store grant handle after checking validity
>>>>
>>>> v3:
>>>> - fix comment in xenvif_tx_dealloc_action()
>>>> - call unmap hypercall directly instead of gnttab_unmap_refs(), which does
>>>>   unnecessary m2p_override. Also remove pages_to_[un]map members
>>>
>>> Is it worthy to have another function call
>>> gnttab_unmap_refs_no_m2p_override in Xen core driver, or just add a
>>> parameter to control wether we need to touch m2p_override? I *think* it
>>> will benefit block driver as well?
>>
>> add_m2p_override and remove_m2p_override calls should be moved into the
>> gntdev device as that should be the only user.
> 
> First of all the gntdev device is common code, while the m2p_override is
> an x86 concept.

m2p_add_override() and m2p_remove_override() are already called from
common code and ARM already provides inline stubs.

The m2p override mechanism is also broken by design (local PFN to
foreign MFN may be many-to-one, but the m2p override only works if local
PFN to foreign MFN is one-to-one). So I want the m2p override to be only
used where it is /currently/ necessary.  I think there should be no new
users of it nor should it be considered a fix for any other use case.

David

^ permalink raw reply

* Re: [net-next 3/7] ixgbe: Use static inlines instead of macros
From: Rustad, Mark D @ 2014-01-09 17:34 UTC (permalink / raw)
  To: Scott Feldman
  Cc: Brown, Aaron F, David Miller, Netdev, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <CE14B815-C10B-4091-A608-6742117FBEF6@cumulusnetworks.com>

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

On Jan 8, 2014, at 12:47 AM, Scott Feldman <sfeldma@cumulusnetworks.com> wrote:

> 
> On Jan 7, 2014, at 11:40 PM, Aaron Brown <aaron.f.brown@intel.com> wrote:
> 
>> From: Mark Rustad <mark.d.rustad@intel.com>
>> 
>> -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg)))
>> +static inline void IXGBE_WRITE_REG(struct ixgbe_hw *hw, u32 reg, u32 value)
> 
> Bummer, now you have a all-caps func name.

Agreed, but this is actually a fairly common condition among drivers that used to use macros. It isn't perfect, but at least it is moving in the right direction. I'd rather leave the case change for a later patch series that does only that or has some reason to touch all of the register access sites.

At least the new accessor I introduced is lower case. :-)

-- 
Mark Rustad, Networking Division, Intel Corporation


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

^ permalink raw reply

* Re: [net-next 2/7] ixgbe: Indicate removal state explicitly
From: Rustad, Mark D @ 2014-01-09 17:29 UTC (permalink / raw)
  To: Scott Feldman
  Cc: Brown, Aaron F, David Miller, Netdev, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <79DAF37B-9AA5-407B-81D2-3EDFD8589254@gmail.com>

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

On Jan 8, 2014, at 12:37 AM, Scott Feldman <sfeldma@gmail.com> wrote:

> 
> On Jan 7, 2014, at 11:40 PM, Aaron Brown <aaron.f.brown@intel.com> wrote:
> 
>> From: Mark Rustad <mark.d.rustad@intel.com>
>> 
>> Add a bit, __IXGBE_REMOVE, to indicate that the module is being
>> removed. The __IXGBE_DOWN bit had been overloaded for this purpose,
>> but that leads to trouble. A few places now check both __IXGBE_DOWN
>> and __IXGBE_REMOVE. Notably, setting either bit will prevent service
>> task execution.
>> 
>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
>> Signed-off-by: Aaron Brown <aaron.f.brown@intel.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 12 ++++++++----
>> 2 files changed, 9 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> index 49531cd..8da263a 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> @@ -798,6 +798,7 @@ enum ixgbe_state_t {
>> 	__IXGBE_TESTING,
>> 	__IXGBE_RESETTING,
>> 	__IXGBE_DOWN,
>> +	__IXGBE_REMOVE,
> 
> __IXGBE_REMOVING?  More consistent with _TESTING, _RESETTING, etc.

And more correct in actual meaning as well. Another good suggestion.

-- 
Mark Rustad, Networking Division, Intel Corporation


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

^ 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