Netdev List
 help / color / mirror / Atom feed
* Re: [net-next 06/12] ixgbe: Hardware Timestamping + PTP Hardware Clock (PHC)
From: Richard Cochran @ 2012-05-10 14:11 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Jacob Keller, netdev, gospo, sassmann
In-Reply-To: <1336632413-19135-7-git-send-email-jeffrey.t.kirsher@intel.com>

Mostly, this looks very good. I do have one concern and a nit, though.

On Wed, May 09, 2012 at 11:46:47PM -0700, Jeff Kirsher wrote:
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 1693ec3..9a83c40 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -789,6 +789,13 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
>  		total_bytes += tx_buffer->bytecount;
>  		total_packets += tx_buffer->gso_segs;
>  
> +#ifdef CONFIG_IXGBE_PTP
> +		if (unlikely(tx_buffer->tx_flags &
> +			     IXGBE_TX_FLAGS_TSTAMP))
> +			ixgbe_ptp_tx_hwtstamp(q_vector,
> +					      tx_buffer->skb);

This looks strangely wrapped.

> +
> +#endif
>  		/* free the skb */
>  		dev_kfree_skb_any(tx_buffer->skb);
>  

...

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
> new file mode 100644
> index 0000000..0b6553e
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c

...

> +/**
> + * ixgbe_ptp_rx_hwtstamp - utility function which checks for RX time stamp
> + * @q_vector: structure containing interrupt and ring information
> + * @skb: particular skb to send timestamp with
> + *
> + * if the timestamp is valid, we convert it into the timecounter ns
> + * value, then store that result into the shhwtstamps structure which
> + * is passed up the network stack
> + */
> +void ixgbe_ptp_rx_hwtstamp(struct ixgbe_q_vector *q_vector,
> +			   struct sk_buff *skb)
> +{
> +	struct ixgbe_adapter *adapter;
> +	struct ixgbe_hw *hw;
> +	struct skb_shared_hwtstamps *shhwtstamps;
> +	u64 regval = 0, ns;
> +	u32 tsyncrxctl;
> +	unsigned long flags;
> +
> +	/* we cannot process timestamps on a ring without a q_vector */
> +	if (!q_vector || !q_vector->adapter)
> +		return;
> +
> +	adapter = q_vector->adapter;
> +	hw = &adapter->hw;
> +
> +	tsyncrxctl = IXGBE_READ_REG(hw, IXGBE_TSYNCRXCTL);
> +	regval |= (u64)IXGBE_READ_REG(hw, IXGBE_RXSTMPL);
> +	regval |= (u64)IXGBE_READ_REG(hw, IXGBE_RXSTMPH) << 32;
> +
> +	/*
> +	 * If this bit is set, then the RX registers contain the time stamp. No
> +	 * other packet will be time stamped until we read these registers, so
> +	 * read the registers to make them available again. Because only one
> +	 * packet can be time stamped at a time, we know that the register
> +	 * values must belong to this one here and therefore we don't need to
> +	 * compare any of the additional attributes stored for it.

I suspect that this assumption is wrong. What happens if the time
stamping logic locks a value but the packet is lost because the ring
is full?

BTW, the IGB driver also has this defect.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH 02/12] selinux: tag avc cache alloc as non-critical
From: Casey Schaufler @ 2012-05-10 14:14 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Andrew Morton, Linux-MM, Linux-Netdev, Linux-NFS, LKML,
	David Miller, Trond Myklebust, Neil Brown, Christoph Hellwig,
	Peter Zijlstra, Mike Christie, Eric B Munson, LSM, SE Linux
In-Reply-To: <1336658065-24851-3-git-send-email-mgorman@suse.de>

On 5/10/2012 6:54 AM, Mel Gorman wrote:
> Failing to allocate a cache entry will only harm performance not
> correctness.  Do not consume valuable reserve pages for something
> like that.

Copying to the LSM and SELinux lists.
 
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Mel Gorman <mgorman@suse.de>
> ---
>  security/selinux/avc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 8ee42b2..75c2977 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -280,7 +280,7 @@ static struct avc_node *avc_alloc_node(void)
>  {
>  	struct avc_node *node;
>  
> -	node = kmem_cache_zalloc(avc_node_cachep, GFP_ATOMIC);
> +	node = kmem_cache_zalloc(avc_node_cachep, GFP_ATOMIC|__GFP_NOMEMALLOC);
>  	if (!node)
>  		goto out;
>  


^ permalink raw reply

* Re: [net-next 07/12] ixgbe: Enable timesync clock-out feature for PPS support on X540
From: Richard Cochran @ 2012-05-10 14:17 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Jacob E Keller, netdev, gospo, sassmann
In-Reply-To: <1336632413-19135-8-git-send-email-jeffrey.t.kirsher@intel.com>

On Wed, May 09, 2012 at 11:46:48PM -0700, Jeff Kirsher wrote:
> From: Jacob E Keller <jacob.e.keller@intel.com>
> 
> This patch enables the PPS system in the PHC framework, by enabling
> the clock-out feature on the X540 device. Causes the SDP0 to be set as
> a 1Hz clock. Also configures the timesync interrupt cause in order to
> report each pulse to the PPS via the PHC framework, which can be used
> for general system clock synchronization. (This allows a stable method
> for tuning the general system time via the on-board SYSTIM register
> based clock.)

Glad to see the PPS output and internal PPS support.

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 9a83c40..1ad6e2a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

...

>  /**
> + * ixgbe_ptp_check_pps_event
> + * @adapter - the private adapter structure
> + * @eicr - the interrupt cause register value
> + *
> + * This function is called by the interrupt routine when checking for
> + * interrupts. It will check and handle a pps event.
> + */
> +void ixgbe_ptp_check_pps_event(struct ixgbe_adapter *adapter, u32 eicr)
> +{
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	struct ptp_clock_event event;
> +
> +	event.type = PTP_CLOCK_PPS;
> +
> +	/* Make sure ptp clock is valid, and PPS event enabled */
> +	if (!adapter->ptp_clock ||
> +	    !(adapter->flags2 & IXGBE_FLAG2_PTP_PPS_ENABLED))
> +		return;
> +
> +	switch (hw->mac.type) {
> +	case ixgbe_mac_X540:
> +		if (eicr & IXGBE_EICR_TIMESYNC)

Since this function is called in every interrupt, I would check this
flag first thing.

> +			ptp_clock_event(adapter->ptp_clock, &event);
> +		break;
> +	default:
> +		break;
> +	}
> +}

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH 02/12] selinux: tag avc cache alloc as non-critical
From: Eric Paris @ 2012-05-10 14:25 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Andrew Morton, Linux-MM, Linux-Netdev, Linux-NFS, LKML,
	David Miller, Trond Myklebust, Neil Brown, Christoph Hellwig,
	Peter Zijlstra, Mike Christie, Eric B Munson
In-Reply-To: <1336658065-24851-3-git-send-email-mgorman@suse.de>

On Thu, May 10, 2012 at 9:54 AM, Mel Gorman <mgorman@suse.de> wrote:
> Failing to allocate a cache entry will only harm performance not
> correctness.  Do not consume valuable reserve pages for something
> like that.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Mel Gorman <mgorman@suse.de>

Acked-by: Eric Paris <eparis@redhat.com>

> ---
>  security/selinux/avc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 8ee42b2..75c2977 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -280,7 +280,7 @@ static struct avc_node *avc_alloc_node(void)
>  {
>        struct avc_node *node;
>
> -       node = kmem_cache_zalloc(avc_node_cachep, GFP_ATOMIC);
> +       node = kmem_cache_zalloc(avc_node_cachep, GFP_ATOMIC|__GFP_NOMEMALLOC);
>        if (!node)
>                goto out;
>
> --
> 1.7.9.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH 1/3] drivers/net: Convert compare_ether_addr to ether_addr_equal
From: Jussi Kivilinna @ 2012-05-10 14:32 UTC (permalink / raw)
  To: Joe Perches
  Cc: ath5k-devel-xDcbHBWguxEUs3QNXV6qNA, Stanislaw Gruszka,
	Roopa Prabhu, Nick-juf53994utBLZpfksSYvnA, Wey-Yi Guy,
	Christian Lamparter, Vasanthakumar Thiagarajan, Andy Gospodarek,
	Jiri Slaby, Christian-juf53994utBLZpfksSYvnA,
	Intel-juf53994utBLZpfksSYvnA, Jay Vosburgh, Jouni Malinen,
	Wireless, Nishank Trivedi, Stanislav Yakovlev, Grant Grundler,
	Johannes Berg, Sony Chacko, John W.  Linville, Chris Metcalf,
	Ben Hutchings, Balasubramanian, Sol
In-Reply-To: <7c9881a67c52c2f218480b6742155b6d6928122d.1336618708.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

Quoting Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>:

> Use the new bool function ether_addr_equal to add
> some clarity and reduce the likelihood for misuse
> of compare_ether_addr for sorting.
>
> Done via cocci script:
>
> $ cat compare_ether_addr.cocci
> @@
> expression a,b;
> @@
> -	!compare_ether_addr(a, b)
> +	ether_addr_equal(a, b)
>
> @@
> expression a,b;
> @@
> -	compare_ether_addr(a, b)
> +	!ether_addr_equal(a, b)
>
> @@
> expression a,b;
> @@
> -	!ether_addr_equal(a, b) == 0
> +	ether_addr_equal(a, b)
>
> @@
> expression a,b;
> @@
> -	!ether_addr_equal(a, b) != 0
> +	!ether_addr_equal(a, b)
>
> @@
> expression a,b;
> @@
> -	ether_addr_equal(a, b) == 0
> +	!ether_addr_equal(a, b)
>
> @@
> expression a,b;
> @@
> -	ether_addr_equal(a, b) != 0
> +	ether_addr_equal(a, b)
>
> @@
> expression a,b;
> @@
> -	!!ether_addr_equal(a, b)
> +	ether_addr_equal(a, b)
>
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/net/bonding/bond_main.c                   |    2 +-
>  drivers/net/ethernet/amd/depca.c                  |    3 ++-
>  drivers/net/ethernet/cisco/enic/enic_main.c       |   12 ++++--------
>  drivers/net/ethernet/dec/ewrk3.c                  |    3 ++-
>  drivers/net/ethernet/dec/tulip/de4x5.c            |    2 +-
>  drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c  |    5 ++---
>  drivers/net/ethernet/sfc/ethtool.c                |    2 +-
>  drivers/net/ethernet/sun/sunvnet.c                |    2 +-
>  drivers/net/ethernet/tile/tilepro.c               |    2 +-
>  drivers/net/ethernet/toshiba/ps3_gelic_wireless.c |    8 ++++----
>  drivers/net/tun.c                                 |    2 +-
>  drivers/net/wireless/at76c50x-usb.c               |    2 +-
>  drivers/net/wireless/ath/ath5k/base.c             |    6 +++---
>  drivers/net/wireless/ath/ath9k/recv.c             |    2 +-
>  drivers/net/wireless/ath/carl9170/rx.c            |    2 +-
>  drivers/net/wireless/ipw2x00/libipw_rx.c          |   16 ++++++++--------
>  drivers/net/wireless/iwlegacy/3945.c              |    4 ++--
>  drivers/net/wireless/iwlegacy/4965-mac.c          |    2 +-
>  drivers/net/wireless/iwlegacy/common.c            |   14 +++++++-------
>  drivers/net/wireless/iwlwifi/iwl-agn-rx.c         |    4 ++--
>  drivers/net/wireless/iwlwifi/iwl-agn-rxon.c       |    8 ++++----
>  drivers/net/wireless/iwlwifi/iwl-agn-sta.c        |    6 +++---
>  drivers/net/wireless/mwl8k.c                      |    2 +-
>  drivers/net/wireless/p54/txrx.c                   |    2 +-
>  drivers/net/wireless/rndis_wlan.c                 |   12 ++++++------
>  drivers/net/wireless/rtlwifi/base.c               |    2 +-
>  drivers/net/wireless/rtlwifi/ps.c                 |    2 +-
>  drivers/net/wireless/rtlwifi/rtl8192ce/trx.c      |   10 +++++-----
>  drivers/net/wireless/rtlwifi/rtl8192cu/mac.c      |   10 +++++-----
>  drivers/net/wireless/rtlwifi/rtl8192de/trx.c      |   11 ++++++-----
>  drivers/net/wireless/rtlwifi/rtl8192se/trx.c      |   11 ++++++-----
>  31 files changed, 85 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c  
> b/drivers/net/bonding/bond_main.c
> index 16dbf53..bbb0043 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1961,7 +1961,7 @@ int bond_release(struct net_device *bond_dev,  
> struct net_device *slave_dev)
>  	write_lock_bh(&bond->lock);
>
>  	if (!bond->params.fail_over_mac) {
> -		if (!compare_ether_addr(bond_dev->dev_addr, slave->perm_hwaddr) &&
> +		if (ether_addr_equal(bond_dev->dev_addr, slave->perm_hwaddr) &&
>  		    bond->slave_cnt > 1)
>  			pr_warning("%s: Warning: the permanent HWaddr of %s - %pM - is  
> still in use by %s. Set the HWaddr of %s to a different address to  
> avoid conflicts.\n",
>  				   bond_dev->name, slave_dev->name,
> diff --git a/drivers/net/ethernet/amd/depca.c  
> b/drivers/net/ethernet/amd/depca.c
> index 86dd957..7f7b99a 100644
> --- a/drivers/net/ethernet/amd/depca.c
> +++ b/drivers/net/ethernet/amd/depca.c
> @@ -1079,7 +1079,8 @@ static int depca_rx(struct net_device *dev)
>  						} else {
>  							lp->pktStats.multicast++;
>  						}
> -					} else if (compare_ether_addr(buf, dev->dev_addr) == 0) {
> +					} else if (ether_addr_equal(buf,
> +								    dev->dev_addr)) {
>  						lp->pktStats.unicast++;
>  					}
>
> diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c  
> b/drivers/net/ethernet/cisco/enic/enic_main.c
> index d7ac6c1..8132c78 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_main.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_main.c
> @@ -944,8 +944,7 @@ static void  
> enic_update_multicast_addr_list(struct enic *enic)
>
>  	for (i = 0; i < enic->mc_count; i++) {
>  		for (j = 0; j < mc_count; j++)
> -			if (compare_ether_addr(enic->mc_addr[i],
> -				mc_addr[j]) == 0)
> +			if (ether_addr_equal(enic->mc_addr[i], mc_addr[j]))
>  				break;
>  		if (j == mc_count)
>  			enic_dev_del_addr(enic, enic->mc_addr[i]);
> @@ -953,8 +952,7 @@ static void  
> enic_update_multicast_addr_list(struct enic *enic)
>
>  	for (i = 0; i < mc_count; i++) {
>  		for (j = 0; j < enic->mc_count; j++)
> -			if (compare_ether_addr(mc_addr[i],
> -				enic->mc_addr[j]) == 0)
> +			if (ether_addr_equal(mc_addr[i], enic->mc_addr[j]))
>  				break;
>  		if (j == enic->mc_count)
>  			enic_dev_add_addr(enic, mc_addr[i]);
> @@ -999,8 +997,7 @@ static void enic_update_unicast_addr_list(struct  
> enic *enic)
>
>  	for (i = 0; i < enic->uc_count; i++) {
>  		for (j = 0; j < uc_count; j++)
> -			if (compare_ether_addr(enic->uc_addr[i],
> -				uc_addr[j]) == 0)
> +			if (ether_addr_equal(enic->uc_addr[i], uc_addr[j]))
>  				break;
>  		if (j == uc_count)
>  			enic_dev_del_addr(enic, enic->uc_addr[i]);
> @@ -1008,8 +1005,7 @@ static void  
> enic_update_unicast_addr_list(struct enic *enic)
>
>  	for (i = 0; i < uc_count; i++) {
>  		for (j = 0; j < enic->uc_count; j++)
> -			if (compare_ether_addr(uc_addr[i],
> -				enic->uc_addr[j]) == 0)
> +			if (ether_addr_equal(uc_addr[i], enic->uc_addr[j]))
>  				break;
>  		if (j == enic->uc_count)
>  			enic_dev_add_addr(enic, uc_addr[i]);
> diff --git a/drivers/net/ethernet/dec/ewrk3.c  
> b/drivers/net/ethernet/dec/ewrk3.c
> index 1879f84..17ae8c6 100644
> --- a/drivers/net/ethernet/dec/ewrk3.c
> +++ b/drivers/net/ethernet/dec/ewrk3.c
> @@ -1016,7 +1016,8 @@ static int ewrk3_rx(struct net_device *dev)
>  							} else {
>  								lp->pktStats.multicast++;
>  							}
> -						} else if (compare_ether_addr(p, dev->dev_addr) == 0) {
> +						} else if (ether_addr_equal(p,
> +									    dev->dev_addr)) {
>  							lp->pktStats.unicast++;
>  						}
>  						lp->pktStats.bins[0]++;		/* Duplicates stats.rx_packets */
> diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c  
> b/drivers/net/ethernet/dec/tulip/de4x5.c
> index 18b106c..d3cd489 100644
> --- a/drivers/net/ethernet/dec/tulip/de4x5.c
> +++ b/drivers/net/ethernet/dec/tulip/de4x5.c
> @@ -1874,7 +1874,7 @@ de4x5_local_stats(struct net_device *dev, char  
> *buf, int pkt_len)
>  	} else {
>  	    lp->pktStats.multicast++;
>  	}
> -    } else if (compare_ether_addr(buf, dev->dev_addr) == 0) {
> +    } else if (ether_addr_equal(buf, dev->dev_addr)) {
>          lp->pktStats.unicast++;
>      }
>
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c  
> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> index 5c47135..46e77a2 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> @@ -1965,7 +1965,7 @@ qlcnic_send_filter(struct qlcnic_adapter *adapter,
>  	__le16 vlan_id = 0;
>  	u8 hindex;
>
> -	if (!compare_ether_addr(phdr->h_source, adapter->mac_addr))
> +	if (ether_addr_equal(phdr->h_source, adapter->mac_addr))
>  		return;
>
>  	if (adapter->fhash.fnum >= adapter->fhash.fmax)
> @@ -2235,8 +2235,7 @@ qlcnic_xmit_frame(struct sk_buff *skb, struct  
> net_device *netdev)
>
>  	if (adapter->flags & QLCNIC_MACSPOOF) {
>  		phdr = (struct ethhdr *)skb->data;
> -		if (compare_ether_addr(phdr->h_source,
> -					adapter->mac_addr))
> +		if (!ether_addr_equal(phdr->h_source, adapter->mac_addr))
>  			goto drop_packet;
>  	}
>
> diff --git a/drivers/net/ethernet/sfc/ethtool.c  
> b/drivers/net/ethernet/sfc/ethtool.c
> index f22f45f..62d4b81 100644
> --- a/drivers/net/ethernet/sfc/ethtool.c
> +++ b/drivers/net/ethernet/sfc/ethtool.c
> @@ -1023,7 +1023,7 @@ static int efx_ethtool_set_class_rule(struct  
> efx_nic *efx,
>  			return -EINVAL;
>
>  		/* Is it a default UC or MC filter? */
> -		if (!compare_ether_addr(mac_mask->h_dest, mac_addr_mc_mask) &&
> +		if (ether_addr_equal(mac_mask->h_dest, mac_addr_mc_mask) &&
>  		    vlan_tag_mask == 0) {
>  			if (is_multicast_ether_addr(mac_entry->h_dest))
>  				rc = efx_filter_set_mc_def(&spec);
> diff --git a/drivers/net/ethernet/sun/sunvnet.c  
> b/drivers/net/ethernet/sun/sunvnet.c
> index 38e3ae9..a108db3 100644
> --- a/drivers/net/ethernet/sun/sunvnet.c
> +++ b/drivers/net/ethernet/sun/sunvnet.c
> @@ -618,7 +618,7 @@ struct vnet_port *__tx_port_find(struct vnet  
> *vp, struct sk_buff *skb)
>  	struct vnet_port *port;
>
>  	hlist_for_each_entry(port, n, hp, hash) {
> -		if (!compare_ether_addr(port->raddr, skb->data))
> +		if (ether_addr_equal(port->raddr, skb->data))
>  			return port;
>  	}
>  	port = NULL;
> diff --git a/drivers/net/ethernet/tile/tilepro.c  
> b/drivers/net/ethernet/tile/tilepro.c
> index 3d501ec..96070e9 100644
> --- a/drivers/net/ethernet/tile/tilepro.c
> +++ b/drivers/net/ethernet/tile/tilepro.c
> @@ -843,7 +843,7 @@ static bool tile_net_poll_aux(struct  
> tile_net_cpu *info, int index)
>  		if (!is_multicast_ether_addr(buf)) {
>  			/* Filter packets not for our address. */
>  			const u8 *mine = dev->dev_addr;
> -			filter = compare_ether_addr(mine, buf);
> +			filter = !ether_addr_equal(mine, buf);
>  		}
>  	}
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c  
> b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
> index 5c14f82..961c832 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
> @@ -1590,8 +1590,8 @@ static void  
> gelic_wl_scan_complete_event(struct gelic_wl_info *wl)
>  		found = 0;
>  		oldest = NULL;
>  		list_for_each_entry(target, &wl->network_list, list) {
> -			if (!compare_ether_addr(&target->hwinfo->bssid[2],
> -						&scan_info->bssid[2])) {
> +			if (ether_addr_equal(&target->hwinfo->bssid[2],
> +					     &scan_info->bssid[2])) {
>  				found = 1;
>  				pr_debug("%s: same BBS found scanned list\n",
>  					 __func__);
> @@ -1691,8 +1691,8 @@ struct gelic_wl_scan_info  
> *gelic_wl_find_best_bss(struct gelic_wl_info *wl)
>
>  		/* If bss specified, check it only */
>  		if (test_bit(GELIC_WL_STAT_BSSID_SET, &wl->stat)) {
> -			if (!compare_ether_addr(&scan_info->hwinfo->bssid[2],
> -						wl->bssid)) {
> +			if (ether_addr_equal(&scan_info->hwinfo->bssid[2],
> +					     wl->bssid)) {
>  				best_bss = scan_info;
>  				pr_debug("%s: bssid matched\n", __func__);
>  				break;
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index bb8c72c..987aeef 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -313,7 +313,7 @@ static int run_filter(struct tap_filter *filter,  
> const struct sk_buff *skb)
>
>  	/* Exact match */
>  	for (i = 0; i < filter->count; i++)
> -		if (!compare_ether_addr(eh->h_dest, filter->addr[i]))
> +		if (ether_addr_equal(eh->h_dest, filter->addr[i]))
>  			return 1;
>
>  	/* Inexact match (multicast only) */
> diff --git a/drivers/net/wireless/at76c50x-usb.c  
> b/drivers/net/wireless/at76c50x-usb.c
> index faa8bcb..5ad74c8 100644
> --- a/drivers/net/wireless/at76c50x-usb.c
> +++ b/drivers/net/wireless/at76c50x-usb.c
> @@ -1751,7 +1751,7 @@ static void at76_mac80211_tx(struct  
> ieee80211_hw *hw, struct sk_buff *skb)
>  	 * following workaround is necessary. If the TX frame is an
>  	 * authentication frame extract the bssid and send the CMD_JOIN. */
>  	if (mgmt->frame_control & cpu_to_le16(IEEE80211_STYPE_AUTH)) {
> -		if (compare_ether_addr(priv->bssid, mgmt->bssid)) {
> +		if (!ether_addr_equal(priv->bssid, mgmt->bssid)) {
>  			memcpy(priv->bssid, mgmt->bssid, ETH_ALEN);
>  			ieee80211_queue_work(hw, &priv->work_join_bssid);
>  			dev_kfree_skb_any(skb);
> diff --git a/drivers/net/wireless/ath/ath5k/base.c  
> b/drivers/net/wireless/ath/ath5k/base.c
> index 49e3b19..0ba81a6 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -462,7 +462,7 @@ void ath5k_vif_iter(void *data, u8 *mac, struct  
> ieee80211_vif *vif)
>  	}
>
>  	if (iter_data->need_set_hw_addr && iter_data->hw_macaddr)
> -		if (compare_ether_addr(iter_data->hw_macaddr, mac) == 0)
> +		if (ether_addr_equal(iter_data->hw_macaddr, mac))
>  			iter_data->need_set_hw_addr = false;
>
>  	if (!iter_data->any_assoc) {
> @@ -1170,7 +1170,7 @@ ath5k_check_ibss_tsf(struct ath5k_hw *ah,  
> struct sk_buff *skb,
>
>  	if (ieee80211_is_beacon(mgmt->frame_control) &&
>  	    le16_to_cpu(mgmt->u.beacon.capab_info) & WLAN_CAPABILITY_IBSS &&
> -	    compare_ether_addr(mgmt->bssid, common->curbssid) == 0) {
> +	    ether_addr_equal(mgmt->bssid, common->curbssid)) {
>  		/*
>  		 * Received an IBSS beacon with the same BSSID. Hardware *must*
>  		 * have updated the local TSF. We have to work around various
> @@ -1234,7 +1234,7 @@ ath5k_update_beacon_rssi(struct ath5k_hw *ah,  
> struct sk_buff *skb, int rssi)
>
>  	/* only beacons from our BSSID */
>  	if (!ieee80211_is_beacon(mgmt->frame_control) ||
> -	    compare_ether_addr(mgmt->bssid, common->curbssid) != 0)
> +	    !ether_addr_equal(mgmt->bssid, common->curbssid))
>  		return;
>
>  	ewma_add(&ah->ah_beacon_rssi_avg, rssi);
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c  
> b/drivers/net/wireless/ath/ath9k/recv.c
> index 544e549..e1fcc68 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -1833,7 +1833,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int  
> flush, bool hp)
>  		if (ieee80211_is_beacon(hdr->frame_control)) {
>  			RX_STAT_INC(rx_beacons);
>  			if (!is_zero_ether_addr(common->curbssid) &&
> -			    !compare_ether_addr(hdr->addr3, common->curbssid))
> +			    ether_addr_equal(hdr->addr3, common->curbssid))
>  				rs.is_mybeacon = true;
>  			else
>  				rs.is_mybeacon = false;
> diff --git a/drivers/net/wireless/ath/carl9170/rx.c  
> b/drivers/net/wireless/ath/carl9170/rx.c
> index dc99030..84b22ee 100644
> --- a/drivers/net/wireless/ath/carl9170/rx.c
> +++ b/drivers/net/wireless/ath/carl9170/rx.c
> @@ -538,7 +538,7 @@ static void carl9170_ps_beacon(struct ar9170  
> *ar, void *data, unsigned int len)
>  		return;
>
>  	/* and only beacons from the associated BSSID, please */
> -	if (compare_ether_addr(hdr->addr3, ar->common.curbssid) ||
> +	if (!ether_addr_equal(hdr->addr3, ar->common.curbssid) ||
>  	    !ar->common.curaid)
>  		return;
>
> diff --git a/drivers/net/wireless/ipw2x00/libipw_rx.c  
> b/drivers/net/wireless/ipw2x00/libipw_rx.c
> index c4955d2..02e0579 100644
> --- a/drivers/net/wireless/ipw2x00/libipw_rx.c
> +++ b/drivers/net/wireless/ipw2x00/libipw_rx.c
> @@ -77,8 +77,8 @@ static struct libipw_frag_entry  
> *libipw_frag_cache_find(struct
>
>  		if (entry->skb != NULL && entry->seq == seq &&
>  		    (entry->last_frag + 1 == frag || frag == -1) &&
> -		    !compare_ether_addr(entry->src_addr, src) &&
> -		    !compare_ether_addr(entry->dst_addr, dst))
> +		    ether_addr_equal(entry->src_addr, src) &&
> +		    ether_addr_equal(entry->dst_addr, dst))
>  			return entry;
>  	}
>
> @@ -245,12 +245,12 @@ static int libipw_is_eapol_frame(struct  
> libipw_device *ieee,
>  	/* check that the frame is unicast frame to us */
>  	if ((fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
>  	    IEEE80211_FCTL_TODS &&
> -	    !compare_ether_addr(hdr->addr1, dev->dev_addr) &&
> -	    !compare_ether_addr(hdr->addr3, dev->dev_addr)) {
> +	    ether_addr_equal(hdr->addr1, dev->dev_addr) &&
> +	    ether_addr_equal(hdr->addr3, dev->dev_addr)) {
>  		/* ToDS frame with own addr BSSID and DA */
>  	} else if ((fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
>  		   IEEE80211_FCTL_FROMDS &&
> -		   !compare_ether_addr(hdr->addr1, dev->dev_addr)) {
> +		   ether_addr_equal(hdr->addr1, dev->dev_addr)) {
>  		/* FromDS frame with own addr as DA */
>  	} else
>  		return 0;
> @@ -523,8 +523,8 @@ int libipw_rx(struct libipw_device *ieee, struct  
> sk_buff *skb,
>
>  	if (ieee->iw_mode == IW_MODE_MASTER && !wds &&
>  	    (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
> -	    IEEE80211_FCTL_FROMDS && ieee->stadev
> -	    && !compare_ether_addr(hdr->addr2, ieee->assoc_ap_addr)) {
> +	    IEEE80211_FCTL_FROMDS && ieee->stadev &&
> +	    ether_addr_equal(hdr->addr2, ieee->assoc_ap_addr)) {
>  		/* Frame from BSSID of the AP for which we are a client */
>  		skb->dev = dev = ieee->stadev;
>  		stats = hostap_get_stats(dev);
> @@ -1468,7 +1468,7 @@ static inline int is_same_network(struct  
> libipw_network *src,
>  	 * as one network */
>  	return ((src->ssid_len == dst->ssid_len) &&
>  		(src->channel == dst->channel) &&
> -		!compare_ether_addr(src->bssid, dst->bssid) &&
> +		ether_addr_equal(src->bssid, dst->bssid) &&
>  		!memcmp(src->ssid, dst->ssid, src->ssid_len));
>  }
>
> diff --git a/drivers/net/wireless/iwlegacy/3945.c  
> b/drivers/net/wireless/iwlegacy/3945.c
> index b25c01b..87e5398 100644
> --- a/drivers/net/wireless/iwlegacy/3945.c
> +++ b/drivers/net/wireless/iwlegacy/3945.c
> @@ -453,10 +453,10 @@ il3945_is_network_packet(struct il_priv *il,  
> struct ieee80211_hdr *header)
>  	switch (il->iw_mode) {
>  	case NL80211_IFTYPE_ADHOC:	/* Header: Dest. | Source    | BSSID */
>  		/* packets to our IBSS update information */
> -		return !compare_ether_addr(header->addr3, il->bssid);
> +		return ether_addr_equal(header->addr3, il->bssid);
>  	case NL80211_IFTYPE_STATION:	/* Header: Dest. | AP{BSSID} | Source */
>  		/* packets to our IBSS update information */
> -		return !compare_ether_addr(header->addr2, il->bssid);
> +		return ether_addr_equal(header->addr2, il->bssid);
>  	default:
>  		return 1;
>  	}
> diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c  
> b/drivers/net/wireless/iwlegacy/4965-mac.c
> index f2baf94..509301a 100644
> --- a/drivers/net/wireless/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/iwlegacy/4965-mac.c
> @@ -2565,7 +2565,7 @@ il4965_find_station(struct il_priv *il, const u8 *addr)
>  	spin_lock_irqsave(&il->sta_lock, flags);
>  	for (i = start; i < il->hw_params.max_stations; i++)
>  		if (il->stations[i].used &&
> -		    (!compare_ether_addr(il->stations[i].sta.sta.addr, addr))) {
> +		    ether_addr_equal(il->stations[i].sta.sta.addr, addr)) {
>  			ret = i;
>  			goto out;
>  		}
> diff --git a/drivers/net/wireless/iwlegacy/common.c  
> b/drivers/net/wireless/iwlegacy/common.c
> index eaf24945..cbf2dc1 100644
> --- a/drivers/net/wireless/iwlegacy/common.c
> +++ b/drivers/net/wireless/iwlegacy/common.c
> @@ -1896,8 +1896,8 @@ il_prep_station(struct il_priv *il, const u8  
> *addr, bool is_ap,
>  		sta_id = il->hw_params.bcast_id;
>  	else
>  		for (i = IL_STA_ID; i < il->hw_params.max_stations; i++) {
> -			if (!compare_ether_addr
> -			    (il->stations[i].sta.sta.addr, addr)) {
> +			if (ether_addr_equal(il->stations[i].sta.sta.addr,
> +					     addr)) {
>  				sta_id = i;
>  				break;
>  			}
> @@ -1926,7 +1926,7 @@ il_prep_station(struct il_priv *il, const u8  
> *addr, bool is_ap,
>
>  	if ((il->stations[sta_id].used & IL_STA_DRIVER_ACTIVE) &&
>  	    (il->stations[sta_id].used & IL_STA_UCODE_ACTIVE) &&
> -	    !compare_ether_addr(il->stations[sta_id].sta.sta.addr, addr)) {
> +	    ether_addr_equal(il->stations[sta_id].sta.sta.addr, addr)) {
>  		D_ASSOC("STA %d (%pM) already added, not adding again.\n",
>  			sta_id, addr);
>  		return sta_id;
> @@ -3744,10 +3744,10 @@ il_full_rxon_required(struct il_priv *il)
>
>  	/* These items are only settable from the full RXON command */
>  	CHK(!il_is_associated(il));
> -	CHK(compare_ether_addr(staging->bssid_addr, active->bssid_addr));
> -	CHK(compare_ether_addr(staging->node_addr, active->node_addr));
> -	CHK(compare_ether_addr
> -	    (staging->wlap_bssid_addr, active->wlap_bssid_addr));
> +	CHK(!ether_addr_equal(staging->bssid_addr, active->bssid_addr));
> +	CHK(!ether_addr_equal(staging->node_addr, active->node_addr));
> +	CHK(!ether_addr_equal(staging->wlap_bssid_addr,
> +			      active->wlap_bssid_addr));
>  	CHK_NEQ(staging->dev_type, active->dev_type);
>  	CHK_NEQ(staging->channel, active->channel);
>  	CHK_NEQ(staging->air_propagation, active->air_propagation);
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c  
> b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
> index 0c252c5..779f819 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
> @@ -779,8 +779,8 @@ static void  
> iwlagn_pass_packet_to_mac80211(struct iwl_priv *priv,
>  	*/
>  	if (unlikely(ieee80211_is_beacon(fc) && priv->passive_no_rx)) {
>  		for_each_context(priv, ctx) {
> -			if (compare_ether_addr(hdr->addr3,
> -					       ctx->active.bssid_addr))
> +			if (!ether_addr_equal(hdr->addr3,
> +					      ctx->active.bssid_addr))
>  				continue;
>  			iwlagn_lift_passive_no_rx(priv);
>  		}
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c  
> b/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c
> index 0f7c444..74fbee6 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c
> @@ -881,10 +881,10 @@ int iwl_full_rxon_required(struct iwl_priv *priv,
>
>  	/* These items are only settable from the full RXON command */
>  	CHK(!iwl_is_associated_ctx(ctx));
> -	CHK(compare_ether_addr(staging->bssid_addr, active->bssid_addr));
> -	CHK(compare_ether_addr(staging->node_addr, active->node_addr));
> -	CHK(compare_ether_addr(staging->wlap_bssid_addr,
> -				active->wlap_bssid_addr));
> +	CHK(!ether_addr_equal(staging->bssid_addr, active->bssid_addr));
> +	CHK(!ether_addr_equal(staging->node_addr, active->node_addr));
> +	CHK(!ether_addr_equal(staging->wlap_bssid_addr,
> +			      active->wlap_bssid_addr));
>  	CHK_NEQ(staging->dev_type, active->dev_type);
>  	CHK_NEQ(staging->channel, active->channel);
>  	CHK_NEQ(staging->air_propagation, active->air_propagation);
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c  
> b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
> index 67e6f1d..b31584e 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
> @@ -322,8 +322,8 @@ u8 iwl_prep_station(struct iwl_priv *priv,  
> struct iwl_rxon_context *ctx,
>  		sta_id = ctx->bcast_sta_id;
>  	else
>  		for (i = IWL_STA_ID; i < IWLAGN_STATION_COUNT; i++) {
> -			if (!compare_ether_addr(priv->stations[i].sta.sta.addr,
> -						addr)) {
> +			if (ether_addr_equal(priv->stations[i].sta.sta.addr,
> +					     addr)) {
>  				sta_id = i;
>  				break;
>  			}
> @@ -353,7 +353,7 @@ u8 iwl_prep_station(struct iwl_priv *priv,  
> struct iwl_rxon_context *ctx,
>
>  	if ((priv->stations[sta_id].used & IWL_STA_DRIVER_ACTIVE) &&
>  	    (priv->stations[sta_id].used & IWL_STA_UCODE_ACTIVE) &&
> -	    !compare_ether_addr(priv->stations[sta_id].sta.sta.addr, addr)) {
> +	    ether_addr_equal(priv->stations[sta_id].sta.sta.addr, addr)) {
>  		IWL_DEBUG_ASSOC(priv, "STA %d (%pM) already added, not "
>  				"adding again.\n", sta_id, addr);
>  		return sta_id;
> diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
> index e30cc32..cf7bdc6 100644
> --- a/drivers/net/wireless/mwl8k.c
> +++ b/drivers/net/wireless/mwl8k.c
> @@ -1235,7 +1235,7 @@ mwl8k_capture_bssid(struct mwl8k_priv *priv,  
> struct ieee80211_hdr *wh)
>  {
>  	return priv->capture_beacon &&
>  		ieee80211_is_beacon(wh->frame_control) &&
> -		!compare_ether_addr(wh->addr3, priv->capture_bssid);
> +		ether_addr_equal(wh->addr3, priv->capture_bssid);
>  }
>
>  static inline void mwl8k_save_beacon(struct ieee80211_hw *hw,
> diff --git a/drivers/net/wireless/p54/txrx.c  
> b/drivers/net/wireless/p54/txrx.c
> index 7c8f118..82a1cac 100644
> --- a/drivers/net/wireless/p54/txrx.c
> +++ b/drivers/net/wireless/p54/txrx.c
> @@ -308,7 +308,7 @@ static void p54_pspoll_workaround(struct  
> p54_common *priv, struct sk_buff *skb)
>  		return;
>
>  	/* only consider beacons from the associated BSSID */
> -	if (compare_ether_addr(hdr->addr3, priv->bssid))
> +	if (!ether_addr_equal(hdr->addr3, priv->bssid))
>  		return;
>
>  	tim = p54_find_ie(skb, WLAN_EID_TIM);
> diff --git a/drivers/net/wireless/rndis_wlan.c  
> b/drivers/net/wireless/rndis_wlan.c
> index d66e298..dcf0e7e 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -1801,8 +1801,8 @@ static struct ndis_80211_pmkid  
> *remove_pmkid(struct usbnet *usbdev,
>  		count = max_pmkids;
>
>  	for (i = 0; i < count; i++)
> -		if (!compare_ether_addr(pmkids->bssid_info[i].bssid,
> -							pmksa->bssid))
> +		if (ether_addr_equal(pmkids->bssid_info[i].bssid,
> +				     pmksa->bssid))
>  			break;
>
>  	/* pmkid not found */
> @@ -1843,8 +1843,8 @@ static struct ndis_80211_pmkid  
> *update_pmkid(struct usbnet *usbdev,
>
>  	/* update with new pmkid */
>  	for (i = 0; i < count; i++) {
> -		if (compare_ether_addr(pmkids->bssid_info[i].bssid,
> -							pmksa->bssid))
> +		if (!ether_addr_equal(pmkids->bssid_info[i].bssid,
> +				      pmksa->bssid))
>  			continue;
>
>  		memcpy(pmkids->bssid_info[i].pmkid, pmksa->pmkid,
> @@ -2139,7 +2139,7 @@ resize_buf:
>  	while (check_bssid_list_item(bssid, bssid_len, buf, len)) {
>  		if (rndis_bss_info_update(usbdev, bssid) && match_bssid &&
>  		    matched) {
> -			if (compare_ether_addr(bssid->mac, match_bssid))
> +			if (!ether_addr_equal(bssid->mac, match_bssid))

While reviewing this, noticed that above original code is wrong. It  
should be !compare_ether_addr. So do I push patch fixing this through  
wireless-testing althought it will later cause conflict with this patch?

-Jussi

>  				*matched = true;
>  		}
>
> @@ -2531,7 +2531,7 @@ static int rndis_get_station(struct wiphy  
> *wiphy, struct net_device *dev,
>  	struct rndis_wlan_private *priv = wiphy_priv(wiphy);
>  	struct usbnet *usbdev = priv->usbdev;
>
> -	if (compare_ether_addr(priv->bssid, mac))
> +	if (!ether_addr_equal(priv->bssid, mac))
>  		return -ENOENT;
>
>  	rndis_fill_station_info(usbdev, sinfo);
> diff --git a/drivers/net/wireless/rtlwifi/base.c  
> b/drivers/net/wireless/rtlwifi/base.c
> index e54488d..f4c852c 100644
> --- a/drivers/net/wireless/rtlwifi/base.c
> +++ b/drivers/net/wireless/rtlwifi/base.c
> @@ -1460,7 +1460,7 @@ void rtl_recognize_peer(struct ieee80211_hw  
> *hw, u8 *data, unsigned int len)
>  		return;
>
>  	/* and only beacons from the associated BSSID, please */
> -	if (compare_ether_addr(hdr->addr3, rtlpriv->mac80211.bssid))
> +	if (!ether_addr_equal(hdr->addr3, rtlpriv->mac80211.bssid))
>  		return;
>
>  	if (rtl_find_221_ie(hw, data, len))
> diff --git a/drivers/net/wireless/rtlwifi/ps.c  
> b/drivers/net/wireless/rtlwifi/ps.c
> index 5b9c3b5..5ae2664 100644
> --- a/drivers/net/wireless/rtlwifi/ps.c
> +++ b/drivers/net/wireless/rtlwifi/ps.c
> @@ -480,7 +480,7 @@ void rtl_swlps_beacon(struct ieee80211_hw *hw,  
> void *data, unsigned int len)
>  		return;
>
>  	/* and only beacons from the associated BSSID, please */
> -	if (compare_ether_addr(hdr->addr3, rtlpriv->mac80211.bssid))
> +	if (!ether_addr_equal(hdr->addr3, rtlpriv->mac80211.bssid))
>  		return;
>
>  	rtlpriv->psc.last_beacon = jiffies;
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c  
> b/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
> index 37b1363..3af874e 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
> @@ -508,14 +508,14 @@ static void  
> _rtl92ce_translate_rx_signal_stuff(struct ieee80211_hw *hw,
>
>  	packet_matchbssid =
>  	    ((IEEE80211_FTYPE_CTL != type) &&
> -	     (!compare_ether_addr(mac->bssid,
> -				  (c_fc & IEEE80211_FCTL_TODS) ?
> -				  hdr->addr1 : (c_fc & IEEE80211_FCTL_FROMDS) ?
> -				  hdr->addr2 : hdr->addr3)) &&
> +	     ether_addr_equal(mac->bssid,
> +			      (c_fc & IEEE80211_FCTL_TODS) ? hdr->addr1 :
> +			      (c_fc & IEEE80211_FCTL_FROMDS) ? hdr->addr2 :
> +			      hdr->addr3) &&
>  	     (!pstats->hwerror) && (!pstats->crc) && (!pstats->icv));
>
>  	packet_toself = packet_matchbssid &&
> -	    (!compare_ether_addr(praddr, rtlefuse->dev_addr));
> +	     ether_addr_equal(praddr, rtlefuse->dev_addr);
>
>  	if (ieee80211_is_beacon(fc))
>  		packet_beacon = true;
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/mac.c  
> b/drivers/net/wireless/rtlwifi/rtl8192cu/mac.c
> index 025bdc2..7e91c76 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192cu/mac.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192cu/mac.c
> @@ -1099,14 +1099,14 @@ void rtl92c_translate_rx_signal_stuff(struct  
> ieee80211_hw *hw,
>  	praddr = hdr->addr1;
>  	packet_matchbssid =
>  	    ((IEEE80211_FTYPE_CTL != type) &&
> -	     (!compare_ether_addr(mac->bssid,
> -			  (cpu_fc & IEEE80211_FCTL_TODS) ?
> -			  hdr->addr1 : (cpu_fc & IEEE80211_FCTL_FROMDS) ?
> -			  hdr->addr2 : hdr->addr3)) &&
> +	     ether_addr_equal(mac->bssid,
> +			      (cpu_fc & IEEE80211_FCTL_TODS) ? hdr->addr1 :
> +			      (cpu_fc & IEEE80211_FCTL_FROMDS) ? hdr->addr2 :
> +			      hdr->addr3) &&
>  	     (!pstats->hwerror) && (!pstats->crc) && (!pstats->icv));
>
>  	packet_toself = packet_matchbssid &&
> -	    (!compare_ether_addr(praddr, rtlefuse->dev_addr));
> +	    ether_addr_equal(praddr, rtlefuse->dev_addr);
>  	if (ieee80211_is_beacon(fc))
>  		packet_beacon = true;
>  	_rtl92c_query_rxphystatus(hw, pstats, pdesc, p_drvinfo,
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/trx.c  
> b/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
> index a7f6126..1666ef7 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
> @@ -466,12 +466,13 @@ static void  
> _rtl92de_translate_rx_signal_stuff(struct ieee80211_hw *hw,
>  	type = WLAN_FC_GET_TYPE(fc);
>  	praddr = hdr->addr1;
>  	packet_matchbssid = ((IEEE80211_FTYPE_CTL != type) &&
> -	     (!compare_ether_addr(mac->bssid, (cfc & IEEE80211_FCTL_TODS) ?
> -		  hdr->addr1 : (cfc & IEEE80211_FCTL_FROMDS) ?
> -		  hdr->addr2 : hdr->addr3)) && (!pstats->hwerror) &&
> -		  (!pstats->crc) && (!pstats->icv));
> +	     ether_addr_equal(mac->bssid,
> +			      (cfc & IEEE80211_FCTL_TODS) ? hdr->addr1 :
> +			      (cfc & IEEE80211_FCTL_FROMDS) ? hdr->addr2 :
> +			      hdr->addr3) &&
> +	     (!pstats->hwerror) && (!pstats->crc) && (!pstats->icv));
>  	packet_toself = packet_matchbssid &&
> -			(!compare_ether_addr(praddr, rtlefuse->dev_addr));
> +			ether_addr_equal(praddr, rtlefuse->dev_addr);
>  	if (ieee80211_is_beacon(fc))
>  		packet_beacon = true;
>  	_rtl92de_query_rxphystatus(hw, pstats, pdesc, p_drvinfo,
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/trx.c  
> b/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
> index 2fd3d13..812b585 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
> @@ -492,13 +492,14 @@ static void  
> _rtl92se_translate_rx_signal_stuff(struct ieee80211_hw *hw,
>  	praddr = hdr->addr1;
>
>  	packet_matchbssid = ((IEEE80211_FTYPE_CTL != type) &&
> -	     (!compare_ether_addr(mac->bssid, (cfc & IEEE80211_FCTL_TODS) ?
> -			hdr->addr1 : (cfc & IEEE80211_FCTL_FROMDS) ?
> -			hdr->addr2 : hdr->addr3)) && (!pstats->hwerror) &&
> -			(!pstats->crc) && (!pstats->icv));
> +	     ether_addr_equal(mac->bssid,
> +			      (cfc & IEEE80211_FCTL_TODS) ? hdr->addr1 :
> +			      (cfc & IEEE80211_FCTL_FROMDS) ? hdr->addr2 :
> +			      hdr->addr3) &&
> +	     (!pstats->hwerror) && (!pstats->crc) && (!pstats->icv));
>
>  	packet_toself = packet_matchbssid &&
> -	    (!compare_ether_addr(praddr, rtlefuse->dev_addr));
> +	    ether_addr_equal(praddr, rtlefuse->dev_addr);
>
>  	if (ieee80211_is_beacon(fc))
>  		packet_beacon = true;
> --
> 1.7.8.111.gad25c.dirty
>
>
>

^ permalink raw reply

* [PATCH] net: device - added support of clearing device statistics
From: Sasikantha babu @ 2012-05-10 15:16 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Michał Mirosław,
	Jiri Pirko, Ben Hutchings
  Cc: netdev, linux-kernel, Sasikantha babu

This patch adds the support of clearing device statistics. Added new 
entry ndo_clear_stats to net_device_ops for device drivers to provide
there own method to clear stats otherwise internal statistics structure
is cleared.

Signed-off-by: Sasikantha babu <sasikanth.v19@gmail.com>
---
 include/linux/netdevice.h |    3 +++
 net/core/dev.c            |   23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5cbaa20..3366bd6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -935,6 +935,8 @@ struct net_device_ops {
 						     struct rtnl_link_stats64 *storage);
 	struct net_device_stats* (*ndo_get_stats)(struct net_device *dev);
 
+	void			(*ndo_clear_stats) (struct net_device *dev);
+
 	int			(*ndo_vlan_rx_add_vid)(struct net_device *dev,
 						       unsigned short vid);
 	int			(*ndo_vlan_rx_kill_vid)(struct net_device *dev,
@@ -2576,6 +2578,7 @@ extern void		dev_load(struct net *net, const char *name);
 extern void		dev_mcast_init(void);
 extern struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
 					       struct rtnl_link_stats64 *storage);
+extern void dev_clear_stats(struct net_device *dev);
 extern void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 				    const struct net_device_stats *netdev_stats);
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 9bb8f87..fc29ea4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5870,6 +5870,29 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
 }
 EXPORT_SYMBOL(dev_get_stats);
 
+/**
+ *	dev_clear_stats	- Clear network device statistics
+ *	@dev: device to clear statistics from
+ *
+ *	Clears network statistics of device.
+ *	The device driver may provide its own method by setting
+ *	dev->netdev_ops->ndo_clear_stats;
+ *	otherwise the internal statistics structure is used.
+ */
+void dev_clear_stats(struct net_device *dev)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	if (ops->ndo_clear_stats)
+		ops->ndo_clear_stats(dev);
+	else
+		memset(&dev->stats, 0, sizeof(dev->stats));
+
+	atomic_long_set(&dev->rx_dropped, 0);
+	return;
+}
+EXPORT_SYMBOL(dev_clear_stats);
+
 struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
 {
 	struct netdev_queue *queue = dev_ingress_queue(dev);
-- 
1.7.3.4

^ permalink raw reply related

* Re: [PATCH] net: device - added support of clearing device statistics
From: Eric Dumazet @ 2012-05-10 15:18 UTC (permalink / raw)
  To: Sasikantha babu
  Cc: David S. Miller, Michał Mirosław, Jiri Pirko,
	Ben Hutchings, netdev, linux-kernel
In-Reply-To: <1336662961-15033-1-git-send-email-sasikanth.v19@gmail.com>

On Thu, 2012-05-10 at 20:46 +0530, Sasikantha babu wrote:
> This patch adds the support of clearing device statistics. Added new 
> entry ndo_clear_stats to net_device_ops for device drivers to provide
> there own method to clear stats otherwise internal statistics structure
> is cleared.
> 
> Signed-off-by: Sasikantha babu <sasikanth.v19@gmail.com>

This is forbidden and racy.

SNMP counters must be increasing.

^ permalink raw reply

* [PATCH net-next] net_sched: update bstats in dequeue()
From: Eric Dumazet @ 2012-05-10 15:36 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Class bytes/packets stats can be misleading because they are updated in
enqueue() while packet might be dropped later.

We already fixed all qdiscs but sch_atm.

This patch makes the final cleanup.

class rate estimators can now match qdisc ones.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_atm.c  |    4 ++--
 net/sched/sch_drr.c  |    4 ++--
 net/sched/sch_hfsc.c |    2 +-
 net/sched/sch_htb.c  |    2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index a77a4fb..8522a47 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -423,8 +423,6 @@ drop: __maybe_unused
 		}
 		return ret;
 	}
-	qdisc_bstats_update(sch, skb);
-	bstats_update(&flow->bstats, skb);
 	/*
 	 * Okay, this may seem weird. We pretend we've dropped the packet if
 	 * it goes via ATM. The reason for this is that the outer qdisc
@@ -472,6 +470,8 @@ static void sch_atm_dequeue(unsigned long data)
 			if (unlikely(!skb))
 				break;
 
+			qdisc_bstats_update(sch, skb);
+			bstats_update(&flow->bstats, skb);
 			pr_debug("atm_tc_dequeue: sending on class %p\n", flow);
 			/* remove any LL header somebody else has attached */
 			skb_pull(skb, skb_network_offset(skb));
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index c218987..9ce0b4f 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -376,8 +376,6 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		cl->deficit = cl->quantum;
 	}
 
-	bstats_update(&cl->bstats, skb);
-
 	sch->q.qlen++;
 	return err;
 }
@@ -403,6 +401,8 @@ static struct sk_buff *drr_dequeue(struct Qdisc *sch)
 			skb = qdisc_dequeue_peeked(cl->qdisc);
 			if (cl->qdisc->q.qlen == 0)
 				list_del(&cl->alist);
+
+			bstats_update(&cl->bstats, skb);
 			qdisc_bstats_update(sch, skb);
 			sch->q.qlen--;
 			return skb;
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 8db3e2c..6c2ec45 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1609,7 +1609,6 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	if (cl->qdisc->q.qlen == 1)
 		set_active(cl, qdisc_pkt_len(skb));
 
-	bstats_update(&cl->bstats, skb);
 	sch->q.qlen++;
 
 	return NET_XMIT_SUCCESS;
@@ -1657,6 +1656,7 @@ hfsc_dequeue(struct Qdisc *sch)
 		return NULL;
 	}
 
+	bstats_update(&cl->bstats, skb);
 	update_vf(cl, qdisc_pkt_len(skb), cur_time);
 	if (realtime)
 		cl->cl_cumul += qdisc_pkt_len(skb);
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index acae5b0..992ab3d 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -574,7 +574,6 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		}
 		return ret;
 	} else {
-		bstats_update(&cl->bstats, skb);
 		htb_activate(q, cl);
 	}
 
@@ -835,6 +834,7 @@ next:
 	} while (cl != start);
 
 	if (likely(skb != NULL)) {
+		bstats_update(&cl->bstats, skb);
 		cl->un.leaf.deficit[level] -= qdisc_pkt_len(skb);
 		if (cl->un.leaf.deficit[level] < 0) {
 			cl->un.leaf.deficit[level] += cl->quantum;

^ permalink raw reply related

* Re: Information leakage from RDS protocol
From: Venkat Venkatsubra @ 2012-05-10 15:38 UTC (permalink / raw)
  To: Jay Fenlason
  Cc: Linus Torvalds, security, eugene, pmatouse, Netdev, David Miller
In-Reply-To: <20120509155709.GA29413@redhat.com>

On 5/9/2012 10:57 AM, Jay Fenlason wrote:
> On Wed, May 09, 2012 at 10:17:57AM -0500, Venkat Venkatsubra wrote:
>> On 5/8/2012 1:22 PM, Jay Fenlason wrote:
>>>> On Tue, May 8, 2012 at 9:10 AM, Jay Fenlason<fenlason@redhat.com>   wrote:
>>>>> recvfrom() on an RDS socket can return the contents of random(?)
>>>>> kernel memory to userspace if it was called with a address
>>>>> length larger than sizeof(struct sockaddr_in). ?rds_recvmsg() also
>>>>> fails to set the addr_len paramater properly before returning, but
>>>>> that's just a bug.
>>>>>
>>>>> There are also a number of cases wher recvfrom() can return an entirely
>>>>> bogus address. ?Anything in rds_recvmsg() that returns a
>>>>> non-negative value but does not go through the
>>>>> ? "sin = (struct sockaddr_in *)msg->msg_name;"
>>>>> code path at the end of the while(1) loop will return up to 128
>>>>> bytes of kernel memory to userspace.
>>>>>
>>>>> Also, on a receive race, the message that was copied to userspace but
>>>>> received by someone else is not zeroed, meaning that if the next
>>>>> message it receives is smaller, the tail of the raced message is
>>>>> leaked. ?I'm not sure how serious this is, but unexpectedly scribbling
>>>>> on userspace memory (even if it is part of a buffer that userspace
>>>>> asked us to write to) should be avoided.
>>>>>
>>> On Tue, May 08, 2012 at 11:04:01AM -0700, Linus Torvalds wrote:
>>>> Please cc David Miller too on these things, and make sure he knows
>>>> there's no embargo or anything (he won't touch it if there is). Maybe
>>>> you don't want public mailing lists, but in general, the more open we
>>>> can be, the better.
>>> Added.  Nobody has said anything about any embargo to me, either
>>> that they want one or that there shouldn't be one.  Personally, I
>>> don't see any reason to embargo this, but I'm not on any
>>> security-response teams.
>>>
>>>> This seems unfortunate, but at least the address thing is limited to
>>>> sizeof(sockaddr_storage) and is kernel stack - which in turn means
>>>> that while it potentially leaks kernel addresses (bad!), it almost
>>>> certainly won't leak anything fundamentally interesting (ie you can't
>>>> read arbitrary kernel memory and find plaintext passwords etc).
>>>>
>>>> I assume the fix is a trivial
>>>>
>>>>    msg->msg_namelen = sizeof(*sin);
>>>>
>>>> in rds_recvmsg() where it sets up the address?
>>> That fixes the case where it actually sets up the address, but won't
>>> fix the cases where it doesn't even do that.  I don't think anyone
>>> ever thought about what the source address should be for a message
>>> that was generated internally by the kernel.  I think the obvious
>>> possibilities are msg_namelen = 0 (no address) and 127.0.0.1
>>>
>>>> I do wonder if maybe recvmsg() should initialize msg_namelen to 0
>>>> instead of the size of the buffer before calling the low-level recvmsg
>>>> function - so that protocols would have to explicitly set the size to
>>>> the right value. But that would need much more validation.
>>> That would require checking/fixing all of the low-level functions,
>>> which will then have to know that the buffer pointed to by msg is at
>>> most sizeof(struct sockaddr_storage) bytes.  I think it's better to
>>> keep the size of the address buffer there, so the low-level functions
>>> can confirm that the address data they're about to stuff in there
>>> won't overflow the buffer.  (That way if we ever change the size of
>>> the buffer, only one place has to change.)
>>>
>>> And the whole rds recieve subsystem needs a bit of a rewrite to close
>>> the information-leaking receive race.  Keeping the semantics correct
>>> in regards to MSG_PEEK and multiple threads reading the socket at the
>>> same time may be tricky.
>>>
>>> 		-- JF
>>>
>> How about adding the suggested "msg->msg_namelen = sizeof(*sin);"
>> line at the top of rds_recvmsg ?
>> And "msg->msg_namelen = 0;" in the below "break;" cases ? I am
>> assuming the apps wouldn't need to look at msg_name in these cases.
>>                  if (!list_empty(&rs->rs_notify_queue)) {
>>                          ret = rds_notify_queue_get(rs, msg);
>>                          break;
>>                  }
>>
>>                  if (rs->rs_cong_notify) {
>>                          ret = rds_notify_cong(rs, msg);
>>                          break;
>>                  }
> Wouldn't it be better to set msg->msg_namelen = 0 at the top of the
> function, and only set it to sizeof(*sin) after msg->msg_name is
> filled in?  That'll prevent accidental disclosure of kernel memory via
> unanticipated code paths.
>
>> And, shouldn't an error be returned for the case below ? Currently
>> zero is returned.
>>
>>          if (msg_flags&  MSG_OOB)
>>                  goto out;
>> An error such as EOPNOTSUPP ?
> I don't know.  I'm not a networking expert.  From what I've found
> googling, EINVAL would be more correct that ENOTSUPP.
>
> This only leaves the datagram contents leak to userspace when multiple
> threads race on receiving a datagram and the subsequent datagram is
> smaller.  That one will be hard to fix, most notably because the
> obvious fixes I've looked at involve losing a datagram if either of
>    inc->i_conn->c_trans->inc_copy_to_user()
> or
>    rds_cmsg_recv()
> fail.  I don't know how likely either of those are, but losing
> datagrams seems like an inappropriate behavior for a reliable datagram
> subsystem.
>
Moving the discussion to netdev.

Venkat

^ permalink raw reply

* Re: [PATCH 07/14] batman-adv: split neigh_new function into generic and batman iv specific parts
From: David Miller @ 2012-05-10 16:07 UTC (permalink / raw)
  To: sven-KaDOiPu9UxWEi8DpZVb4nw
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
	lindner_marek-LWAfsSFWpa4
In-Reply-To: <9033916.UuGU18BIPo@bentobox>

From: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
Date: Thu, 10 May 2012 09:34:25 +0200

> On Wednesday, May 09, 2012 08:41:11 PM David Miller wrote:
> [...]
>> The namespace pollution of the batman-adv code needs to improve,
>> and I'm putting my foot down starting with this change.
>> 
>> If you have a static function which is therefore private to a
>> source file, name it whatever you want.
>> 
>> But once it gets exported out of that file, you have to give it
>> an appropriate name.  Probably with a "batman_adv_" prefix or
>> similar.
> 
> I aggree, but would like to like to have a shorter prefix batadv_. I know that 
> you said "or similar" but there are still some developers that fear your 
> response to a patch that only adds the prefix batadv_ instead of the longer 
> version.

batadv_ is fine.

^ permalink raw reply

* Re: [PATCH 1/3] drivers/net: Convert compare_ether_addr to ether_addr_equal
From: Joe Perches @ 2012-05-10 16:11 UTC (permalink / raw)
  To: Jussi Kivilinna
  Cc: David S. Miller, John W. Linville, netdev, linux-kernel,
	linux-wireless
In-Reply-To: <20120510173201.16131du3cg90nybo@www.81.fi>

(cc's trimmed)

On Thu, 2012-05-10 at 17:32 +0300, Jussi Kivilinna wrote:
> Quoting Joe Perches <joe@perches.com>:
> > Use the new bool function ether_addr_equal to add
> > some clarity and reduce the likelihood for misuse
> > of compare_ether_addr for sorting.
[]
> > diff --git a/drivers/net/wireless/rndis_wlan.c
[]
> > @@ -2139,7 +2139,7 @@ resize_buf:
> >  	while (check_bssid_list_item(bssid, bssid_len, buf, len)) {
> >  		if (rndis_bss_info_update(usbdev, bssid) && match_bssid &&
> >  		    matched) {
> > -			if (compare_ether_addr(bssid->mac, match_bssid))
> > +			if (!ether_addr_equal(bssid->mac, match_bssid))
> 
> While reviewing this, noticed that above original code is wrong. It  
> should be !compare_ether_addr. So do I push patch fixing this through  
> wireless-testing althought it will later cause conflict with this patch?
> 
> -Jussi
> 
> >  				*matched = true;
> >  		}
> >

Up to John.

Here's the patch I would send against net-next
updating the test and the style a little.

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index dcf0e7e..29cccc5 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -2137,11 +2137,10 @@ resize_buf:
 	 * received 'num_items' and walking through full bssid buffer instead.
 	 */
 	while (check_bssid_list_item(bssid, bssid_len, buf, len)) {
-		if (rndis_bss_info_update(usbdev, bssid) && match_bssid &&
-		    matched) {
-			if (!ether_addr_equal(bssid->mac, match_bssid))
-				*matched = true;
-		}
+		if (rndis_bss_info_update(usbdev, bssid) &&
+		    match_bssid && matched &&
+		    ether_addr_equal(bssid->mac, match_bssid))
+			*matched = true;
 
 		real_count++;
 		bssid = next_bssid_list_item(bssid, &bssid_len, buf, len);

^ permalink raw reply related

* Re: [PATCH 1/2 net] 6lowpan: add missing pskb_may_pull() check
From: David Miller @ 2012-05-10 16:28 UTC (permalink / raw)
  To: alex.bluesman.smirnov; +Cc: eric.dumazet, netdev
In-Reply-To: <CAJmB2rCo0CfHP992Jix=mtshSS-wn92rf+3yg-ZK1_KLkXKRGw@mail.gmail.com>

From: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
Date: Thu, 10 May 2012 18:05:46 +0400

> Using BUG() macro I just want to indicate that something in the bottom
> of the stack went terribly wrong and you must check your code for
> bugs..

Then you should do something like:

	if (WARN_ON_ONCE(!pskb_may_pull(...))) {
		appropriate_error_handling();
		return;
	}

instead.

^ permalink raw reply

* Re: [PATCH 1/3] drivers/net: Convert compare_ether_addr to ether_addr_equal
From: David Miller @ 2012-05-10 16:30 UTC (permalink / raw)
  To: jussi.kivilinna
  Cc: joe, fubar, andy, benve, roprabhu, neepatel, nistrive, grundler,
	anirban.chakraborty, sony.chacko, linux-driver, linux-net-drivers,
	bhutchings, cmetcalf, linville, jirislaby, mickflemm, mcgrof,
	jouni, vthiagar, senthilb, chunkeey, stas.yakovlev, sgruszka,
	johannes.berg, wey-yi.w.guy, ilw, buytenh, Larry.Finger,
	chaoming_li, netdev, linux-kernel, linux-wireless, ath5k-devel,
	ath9k-devel
In-Reply-To: <20120510173201.16131du3cg90nybo@www.81.fi>


Never, EVER, quote an entire large patch just to make a comment
on one small hunk.

I very nearly missed what you had to say because when scrolling
through it it appeared as if you made no comments at all.

Again, NEVER, EVER, do this.  It's extremely anti-social.  Edit out
the irrelevant quoted content when replying to people, always.

^ permalink raw reply

* Re: [PATCH 1/3] drivers/net: Convert compare_ether_addr to ether_addr_equal
From: David Miller @ 2012-05-10 16:33 UTC (permalink / raw)
  To: joe; +Cc: jussi.kivilinna, linville, netdev, linux-kernel, linux-wireless
In-Reply-To: <1336666288.22495.14.camel@joe2Laptop>

From: Joe Perches <joe@perches.com>
Date: Thu, 10 May 2012 09:11:28 -0700

> (cc's trimmed)
> 
> On Thu, 2012-05-10 at 17:32 +0300, Jussi Kivilinna wrote:
>> Quoting Joe Perches <joe@perches.com>:
>> > Use the new bool function ether_addr_equal to add
>> > some clarity and reduce the likelihood for misuse
>> > of compare_ether_addr for sorting.
> []
>> > diff --git a/drivers/net/wireless/rndis_wlan.c
> []
>> > @@ -2139,7 +2139,7 @@ resize_buf:
>> >  	while (check_bssid_list_item(bssid, bssid_len, buf, len)) {
>> >  		if (rndis_bss_info_update(usbdev, bssid) && match_bssid &&
>> >  		    matched) {
>> > -			if (compare_ether_addr(bssid->mac, match_bssid))
>> > +			if (!ether_addr_equal(bssid->mac, match_bssid))
>> 
>> While reviewing this, noticed that above original code is wrong. It  
>> should be !compare_ether_addr. So do I push patch fixing this through  
>> wireless-testing althought it will later cause conflict with this patch?
>> 
>> -Jussi
>> 
>> >  				*matched = true;
>> >  		}
>> >
> 
> Up to John.
> 
> Here's the patch I would send against net-next
> updating the test and the style a little.

I think in this specific case it's better to push this one directly
through net-next.  But yes, it's up to John.

^ permalink raw reply

* Re: Netlink for kernel<->user space communication?
From: Stephen Hemminger @ 2012-05-10 16:36 UTC (permalink / raw)
  To: Arvid Brodin; +Cc: netdev@vger.kernel.org
In-Reply-To: <4FAAFE76.9060508@xdin.com>

On Wed, 9 May 2012 23:32:08 +0000
Arvid Brodin <Arvid.Brodin@xdin.com> wrote:

> On 2012-05-08 00:33, Stephen Hemminger wrote:
> > On Mon, 7 May 2012 18:43:23 +0000
> > Arvid Brodin <Arvid.Brodin@xdin.com> wrote:
> > 
> >> On Tue, 24 Apr 2012 16:57:55 -0700
> >> Stephen Hemminger <shemminger@xxxxxxxxxx> wrote:
> >>> On Tue, 24 Apr 2012 23:52:34 +0000
> >>> Arvid Brodin <Arvid.Brodin@xxxxxxxx> wrote:
> >>>
> >>>> Hi.
> >>>>
> >>>> I'm writing a kernel driver for the HSR protocol, a standard for high availability
> >>>> networks. I want to send messages from the kernel to user space about broken network
> >>>> links. I also want user space to be able to ask the kernel about its view of the status of
> >>>> nodes on the network.
> >>>>
> >>>> Netlink seems like a good tool for this. (Is it?)
> >>>
> >>> Yes.
> >>>
> >>>> But do I use raw netlink? (Described here: http://www.linuxjournal.com/article/7356 - but
> >>>> this seems a bit out of date, the kernel API description differs from today's kernel
> >>>> implementation.)
> >>>
> >>> No. Your driver probably looks like a device so you should be
> >>> using rtnetlink messages.
> >>
> >> I'm already using rtnetlink messages to add and remove my device, which works fine (see
> >> e.g. http://www.spinics.net/lists/netdev/msg192817.html - although I didn't think it
> >> meaningful to include the iproute2 patch here, until the kernel part is ready).
> >>
> >> The protocol specifies transmission of "supervision frames" every 2 seconds, e.g. to check
> >> link integrity. Every such frame should be received from two directions in the ring - if
> >> only one is received, then there is a link problem.
> > 
> > Why not just manipulate the carrier or operational state (see Documentation/networking/operstate)
> > and use the existing notification on link changes. If you don't get heartbeat then change
> > the state of the device to indicate lower device is down with set_operstate(), the necessary
> > link everts propgate back as netlink events.
> 
> With HSR, all nodes in the network ring can detect a link problem anywhere in the ring. So
> I need a way to communicate link problems that does not concern the host's devices at all,
> but rather the state of the network as a whole. A typical message might say "Frames from
> node 01:23:45:67:89:AB is only received over Slave Interface 1!" This indicates a problem
> since all frames should be received over both slave interfaces. The broken link can be
> anywhere between this node and the indicated node. If user space is aware of the network
> topology, it can figure out exactly where the damage is by looking at which nodes' frames
> are received over which slave interface.
> 
> (Thanks for the operstates info though, I hadn't discovered IF_OPER_LOWERLAYERDOWN! I will
> use it to indicate a local slave is down.)

Sounds like a message that is specific to the protocol. Maybe just a log
message would suffice, or having a protocol specific event channel.

> >> I'd like to notify user space about every such occurence. Is there a rtnetlink message
> >> type that fits this? The stuff in rtnetlink.h seems to be mostly concerned with specific
> >> user space commands (there is something called RTNLGRP_NOTIFY but I couldn't find any
> >> instances of it being used in the kernel, nor any documentation).
> >>
> > 
> > I am trying to steer you to use existing API's because then existing programs and
> > infrastructure can deal with the new device type.
> 
> I really appreciate that! I want to use existing API's as far as possible. That's why I
> keep sending you all these questions. :)
> 
> 

^ permalink raw reply

* Re: [PATCH] netlink: connector: implement cn_netlink_reply
From: Evgeniy Polyakov @ 2012-05-10 16:47 UTC (permalink / raw)
  To: Alban Crequy
  Cc: Ben Hutchings, netdev, Vincent Sanders, Javier Martinez Canillas,
	Rodrigo Moya
In-Reply-To: <20120510113908.5b79c5f3@chocolatine.cbg.collabora.co.uk>

On Thu, May 10, 2012 at 11:39:08AM +0100, Alban Crequy (alban.crequy@collabora.co.uk) wrote:
> The code to use the feature is not yet ready for submission and we will
> add this patch to the front of that submission in due course.
> 
> We are just being good community members and making each patch
> available early. Thanks for your feedback on this patch. Please let
> me know if I can add any reviewed-by.

Yes, you can add review tag with your submission for connector, your
changes look good

-- 
	Evgeniy Polyakov

^ permalink raw reply

* Re: [PATCH 0/3] staging: delete duplicated files/functionality in rtl8712
From: Greg Kroah-Hartman @ 2012-05-10 16:54 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: linux-wireless, florian.c.schilhabel, ali, netdev, Larry Finger
In-Reply-To: <1336621641-15467-1-git-send-email-paul.gortmaker@windriver.com>

On Wed, May 09, 2012 at 11:47:18PM -0400, Paul Gortmaker wrote:
> A git grep happened to lead me into this dir, and once there I couldn't
> simply leave and pretend I didn't see the stuff that I saw.
> 
> There were duplicated basic networking headers like if_ether.h and
> ip.h (of course, some ancient versions, too).  And a whole whack of
> boilerplate endian handling functionality duplicated too.
> 
> I forced myself to stop looking after that.

Yes, it's a mess, thanks for working on cleaning it up, even a little
bit, it all helps.

> Anyway, a trivial redirection onto mainline's networking headers
> in the proper include dir, and a deletion of any references to
> the endian headers and the thing still happily builds on x86_64
> and ppc even after shitcanning seven useless files.
> 
> These three commits are against May 8th's linux-next tree.  I've
> used "-D" to hide the line-by-line content of the deletions.

Can you regenerate these with the full deletion content?  There have
been some spelling fixes in these files, and your patches do not apply
due to them.  I can hand-edit the patches if you regenerate it.

Or if you rebase on the last linux-next tree, that will pick those
changes up and then I can apply these properly.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 00/17] Swap-over-NBD without deadlocking V10
From: Mike Christie @ 2012-05-10 17:17 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Andrew Morton, Linux-MM, Linux-Netdev, LKML, David Miller,
	Neil Brown, Peter Zijlstra, Eric B Munson
In-Reply-To: <1336657510-24378-1-git-send-email-mgorman@suse.de>

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

On 05/10/2012 08:44 AM, Mel Gorman wrote:
> When a user or administrator requires swap for their application, they
> create a swap partition and file, format it with mkswap and activate it
> with swapon. Swap over the network is considered as an option in diskless
> systems. The two likely scenarios are when blade servers are used as part
> of a cluster where the form factor or maintenance costs do not allow the
> use of disks and thin clients.

Thank you for working on this. I made the attached patch for software
iscsi which has the same issue as nbd.

I tested the patch here and did not notice any performance regressions
or any other bugs.

[-- Attachment #2: 0001-iscsi-Set-SOCK_MEMALLOC-for-access-to-PFMEMALLOC-res.patch --]
[-- Type: text/plain, Size: 1585 bytes --]

>From 917d53f16d1e26b12e90e5e15df76a7a8bee35e8 Mon Sep 17 00:00:00 2001
From: Mike Christie <michaelc@cs.wisc.edu>
Date: Thu, 3 May 2012 12:36:18 -0500
Subject: [PATCH 1/1] iscsi: Set SOCK_MEMALLOC for access to PFMEMALLOC
 reserves

Set SOCK_MEMALLOC on the iscsi socket to allow access to PFMEMALLOC
reserves to prevent deadlock.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/iscsi_tcp.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 453a740..7360f4c 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -370,17 +370,24 @@ static inline int iscsi_sw_tcp_xmit_qlen(struct iscsi_conn *conn)
 static int iscsi_sw_tcp_pdu_xmit(struct iscsi_task *task)
 {
 	struct iscsi_conn *conn = task->conn;
-	int rc;
+	unsigned long pflags = current->flags;
+	int rc = 0;
+
+	current->flags |= PF_MEMALLOC;
 
 	while (iscsi_sw_tcp_xmit_qlen(conn)) {
 		rc = iscsi_sw_tcp_xmit(conn);
-		if (rc == 0)
-			return -EAGAIN;
+		if (rc == 0) {
+			rc = -EAGAIN;
+			break;
+		}
 		if (rc < 0)
-			return rc;
+			break;
+		rc = 0;
 	}
 
-	return 0;
+	tsk_restore_flags(current, pflags, PF_MEMALLOC);
+	return rc;
 }
 
 /*
@@ -665,6 +672,7 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
 	sk->sk_reuse = 1;
 	sk->sk_sndtimeo = 15 * HZ; /* FIXME: make it configurable */
 	sk->sk_allocation = GFP_ATOMIC;
+	sk_set_memalloc(sk);
 
 	iscsi_sw_tcp_conn_set_callbacks(conn);
 	tcp_sw_conn->sendpage = tcp_sw_conn->sock->ops->sendpage;
-- 
1.7.7.6


^ permalink raw reply related

* Re: [PATCH 0/3] staging: delete duplicated files/functionality in rtl8712
From: Paul Gortmaker @ 2012-05-10 17:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	florian.c.schilhabel-gM/Ye1E23mwN+BqQ9rBEUg,
	ali-H4aWS73dXuqdkR0RstYxLQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	Larry Finger
In-Reply-To: <20120510165451.GA12958-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

[Re: [PATCH 0/3] staging: delete duplicated files/functionality in rtl8712] On 10/05/2012 (Thu 09:54) Greg Kroah-Hartman wrote:

[...]

> Can you regenerate these with the full deletion content?  There have
> been some spelling fixes in these files, and your patches do not apply
> due to them.  I can hand-edit the patches if you regenerate it.
> 
> Or if you rebase on the last linux-next tree, that will pick those
> changes up and then I can apply these properly.

I've rebased it onto your staging-next of today, so it should be
hopefully seamless for you now.

Thanks,
Paul.
---

The following changes since commit 0e545f6d165ad0180b0d04fe1291f6f6268047ff:

  staging: rts5139: remove unused variable option.ww_enable (2012-05-10 09:57:21 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git rtl8712-staging-next

for you to fetch changes up to 9a0fbbb52ab08017ac51aceef76514799837f4b0:

  staging: delete all duplicated endian crap from rtl8712 driver (2012-05-10 13:10:03 -0400)

----------------------------------------------------------------
Paul Gortmaker (3):
      staging: wean rtl8712 off of its ancient duplicate of if_ether.h
      staging: wean rtl8712 off of its ancient duplicate of ip.h
      staging: delete all duplicated endian crap from rtl8712 driver

 drivers/staging/rtl8712/big_endian.h        |   94 --------------
 drivers/staging/rtl8712/generic.h           |  178 ---------------------------
 drivers/staging/rtl8712/hal_init.c          |    1 -
 drivers/staging/rtl8712/if_ether.h          |  141 ---------------------
 drivers/staging/rtl8712/ip.h                |  137 --------------------
 drivers/staging/rtl8712/little_endian.h     |   94 --------------
 drivers/staging/rtl8712/osdep_service.h     |    2 -
 drivers/staging/rtl8712/rtl8712_cmd.c       |    1 -
 drivers/staging/rtl8712/rtl8712_recv.c      |    5 +-
 drivers/staging/rtl8712/rtl8712_xmit.c      |    1 -
 drivers/staging/rtl8712/rtl871x_byteorder.h |   32 -----
 drivers/staging/rtl8712/rtl871x_cmd.c       |    1 -
 drivers/staging/rtl8712/rtl871x_recv.c      |    4 +-
 drivers/staging/rtl8712/rtl871x_xmit.c      |    1 -
 drivers/staging/rtl8712/swab.h              |  131 --------------------
 drivers/staging/rtl8712/usb_ops.c           |    1 -
 drivers/staging/rtl8712/wifi.h              |    1 -
 drivers/staging/rtl8712/xmit_linux.c        |    6 +-
 18 files changed, 7 insertions(+), 824 deletions(-)
 delete mode 100644 drivers/staging/rtl8712/big_endian.h
 delete mode 100644 drivers/staging/rtl8712/generic.h
 delete mode 100644 drivers/staging/rtl8712/if_ether.h
 delete mode 100644 drivers/staging/rtl8712/ip.h
 delete mode 100644 drivers/staging/rtl8712/little_endian.h
 delete mode 100644 drivers/staging/rtl8712/rtl871x_byteorder.h
 delete mode 100644 drivers/staging/rtl8712/swab.h

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH RFC 1/6] skbuff: support per-page destructors in copy_ubufs
From: Ian Campbell @ 2012-05-10 17:46 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David Miller, netdev@vger.kernel.org, eric.dumazet@gmail.com
In-Reply-To: <8a3235fbceef37758ef23169c4c152e8d1251d3b.1336397823.git.mst@redhat.com>

On Mon, 2012-05-07 at 14:54 +0100, Michael S. Tsirkin wrote:

>  	/* skb frags point to kernel buffers */
>  	for (i = skb_shinfo(skb)->nr_frags; i > 0; i--) {
> +		skb_frag_t *f = &skb_shinfo(skb)->frags[i];

This needs to be ....->frags[i - 1]

otherwise you put every new frag one too high and don't do anything to
frag 0, which leaves the old destructor pointer in place and leads to a
double free.

I think skb_frag_set_destructor and skb_copy_frag_destructor need to
clear and propagate respectively (or maybe just clear in both cases) the
destructor_arg field since it is otherwise not initialised when we set
SKBTX_DEV_ZEROCOPY and that can trigger wrong behaviour in this
function.

Ian.

> +		if (unlikely((!uarg && !f->page.destructor)))
> +			continue;
>  		__skb_fill_page_desc(skb, i-1, head, 0,
>  				     skb_shinfo(skb)->frags[i - 1].size);
>  		head = (struct page *)head->private;

^ permalink raw reply

* [PATCH v13 net-next] codel: Controlled Delay AQM
From: Eric Dumazet @ 2012-05-10 17:51 UTC (permalink / raw)
  To: Dave Täht, David Miller
  Cc: Kathleen Nichols, Van Jacobson, codel, bloat, netdev, Tom Herbert,
	Yuchung Cheng, Matt Mathis, Stephen Hemminger
In-Reply-To: <1336571439.12504.27.camel@edumazet-glaptop>

From: Eric Dumazet <edumazet@google.com>

An implementation of CoDel AQM, from Kathleen Nichols and Van Jacobson. 

http://queue.acm.org/detail.cfm?id=2209336 

This AQM main input is no longer queue size in bytes or packets, but the
delay packets stay in (FIFO) queue.

As we don't have infinite memory, we still can drop packets in enqueue()
in case of massive load, but mean of CoDel is to drop packets in
dequeue(), using a control law based on two simple parameters :

target : target sojourn time (default 5ms)
interval : width of moving time window (default 100ms)

Based on initial work from Dave Taht.

Refactored to help future codel inclusion as a plugin for other linux
qdisc (FQ_CODEL, ...), like RED.

include/net/codel.h contains codel algorithm as close as possible than
Kathleen reference.

net/sched/sch_codel.c contains the linux qdisc specific glue.

Separate structures permit a memory efficient implementation of fq_codel
(to be sent as a separate work) : Each flow has its own struct
codel_vars.

timestamps are taken at enqueue() time with 1024 ns precision, allowing
a range of 2199 seconds in queue, and 100Gb links support. iproute2 uses
usec as base unit.

Selected packets are dropped, unless ECN is enabled and packets can get
ECN mark instead.

Tested from 2Mb to 10Gb speeds with no particular problems, on ixgbe and
tg3 drivers (BQL enabled).

Usage: tc qdisc ... codel [ limit PACKETS ] [ target TIME ]
                          [ interval TIME ] [ ecn ]

qdisc codel 10: parent 1:1 limit 2000p target 3.0ms interval 60.0ms ecn 
 Sent 13347099587 bytes 8815805 pkt (dropped 0, overlimits 0 requeues 0) 
 rate 202365Kbit 16708pps backlog 113550b 75p requeues 0 
  count 116 lastcount 98 ldelay 4.3ms dropping drop_next 816us
  maxpacket 1514 ecn_mark 84399 drop_overlimit 0

CoDel must be seen as a base module, and should be used keeping in mind
there is still a FIFO queue. So a typical setup will probably need a
hierarchy of several qdiscs and packet classifiers to be able to meet
whatever constraints a user might have.

One possible example would be to use fq_codel, which combines Fair
Queueing and CoDel, in replacement of sfq / sfq_red.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Dave Taht <dave.taht@bufferbloat.net>
Cc: Kathleen Nichols <nichols@pollere.com>
Cc: Van Jacobson <van@pollere.net>
Cc: Tom Herbert <therbert@google.com>
Cc: Matt Mathis <mattmathis@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
---
 include/linux/pkt_sched.h |   26 ++
 include/net/codel.h       |  334 ++++++++++++++++++++++++++++++++++++
 net/sched/Kconfig         |   11 +
 net/sched/Makefile        |    1 
 net/sched/sch_codel.c     |  275 +++++++++++++++++++++++++++++
 5 files changed, 647 insertions(+)

diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index ffe975c..cde56c2 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -655,4 +655,30 @@ struct tc_qfq_stats {
 	__u32 lmax;
 };
 
+/* CODEL */
+
+enum {
+	TCA_CODEL_UNSPEC,
+	TCA_CODEL_TARGET,
+	TCA_CODEL_LIMIT,
+	TCA_CODEL_INTERVAL,
+	TCA_CODEL_ECN,
+	__TCA_CODEL_MAX
+};
+
+#define TCA_CODEL_MAX	(__TCA_CODEL_MAX - 1)
+
+struct tc_codel_xstats {
+	__u32	maxpacket; /* largest packet we've seen so far */
+	__u32	count;	   /* how many drops we've done since the last time we
+			    * entered dropping state
+			    */
+	__u32	lastcount; /* count at entry to dropping state */
+	__u32	ldelay;    /* in-queue delay seen by most recently dequeued packet */
+	__s32	drop_next; /* time to drop next packet */
+	__u32	drop_overlimit; /* number of time max qdisc packet limit was hit */
+	__u32	ecn_mark;  /* number of packets we ECN marked instead of dropped */
+	__u32	dropping;  /* are we in dropping state ? */
+};
+
 #endif
diff --git a/include/net/codel.h b/include/net/codel.h
new file mode 100644
index 0000000..ecafb0b
--- /dev/null
+++ b/include/net/codel.h
@@ -0,0 +1,334 @@
+#ifndef __NET_SCHED_CODEL_H
+#define __NET_SCHED_CODEL_H
+
+/*
+ * Codel - The Controlled-Delay Active Queue Management algorithm
+ *
+ *  Copyright (C) 2011-2012 Kathleen Nichols <nichols@pollere.com>
+ *  Copyright (C) 2011-2012 Van Jacobson <van@pollere.net>
+ *  Copyright (C) 2012 Michael D. Taht <dave.taht@bufferbloat.net>
+ *  Copyright (C) 2012 Eric Dumazet <edumazet@google.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions, and the following disclaimer,
+ *    without modification.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The names of the authors may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * Alternatively, provided that this notice is retained in full, this
+ * software may be distributed under the terms of the GNU General
+ * Public License ("GPL") version 2, in which case the provisions of the
+ * GPL apply INSTEAD OF those given above.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/ktime.h>
+#include <linux/skbuff.h>
+#include <net/pkt_sched.h>
+#include <net/inet_ecn.h>
+
+/* Controlling Queue Delay (CoDel) algorithm 
+ * =========================================
+ * Source : Kathleen Nichols and Van Jacobson
+ * http://queue.acm.org/detail.cfm?id=2209336
+ *
+ * Implemented on linux by Dave Taht and Eric Dumazet
+ */
+
+
+/*
+ * CoDel uses a 1024 nsec clock, encoded in u32
+ * This gives a range of 2199 seconds, because of signed compares
+ */
+typedef u32 codel_time_t;
+typedef s32 codel_tdiff_t;
+#define CODEL_SHIFT 10
+#define MS2TIME(a) ((a * NSEC_PER_MSEC) >> CODEL_SHIFT)
+
+static inline codel_time_t codel_get_time(void)
+{
+	u64 ns = ktime_to_ns(ktime_get());
+
+	return ns >> CODEL_SHIFT;
+}
+
+#define codel_time_after(a, b)		((s32)(a) - (s32)(b) > 0)
+#define codel_time_after_eq(a, b)	((s32)(a) - (s32)(b) >= 0)
+#define codel_time_before(a, b)		((s32)(a) - (s32)(b) < 0)
+#define codel_time_before_eq(a, b)	((s32)(a) - (s32)(b) <= 0)
+
+/* Qdiscs using codel plugin must use codel_skb_cb in their own cb[] */
+struct codel_skb_cb {
+	codel_time_t enqueue_time;
+};
+
+static struct codel_skb_cb *get_codel_cb(const struct sk_buff *skb)
+{
+	qdisc_cb_private_validate(skb, sizeof(struct codel_skb_cb));
+	return (struct codel_skb_cb *)qdisc_skb_cb(skb)->data;
+}
+
+static codel_time_t codel_get_enqueue_time(const struct sk_buff *skb)
+{
+	return get_codel_cb(skb)->enqueue_time;
+}
+
+static void codel_set_enqueue_time(struct sk_buff *skb)
+{
+	get_codel_cb(skb)->enqueue_time = codel_get_time();
+}
+
+static inline u32 codel_time_to_us(codel_time_t val)
+{
+	u64 valns = ((u64)val << CODEL_SHIFT);
+
+	do_div(valns, NSEC_PER_USEC);
+	return (u32)valns;
+}
+
+/**
+ * struct codel_params - contains codel parameters
+ * @target:	target queue size (in time units)
+ * @interval:	width of moving time window
+ * @ecn:	is Explicit Congestion Notification enabled
+ */
+struct codel_params {
+	codel_time_t	target;
+	codel_time_t	interval;
+	bool		ecn;
+};
+
+/**
+ * struct codel_vars - contains codel variables
+ * @count:		how many drops we've done since the last time we
+ *			entered dropping state
+ * @lastcount:		count at entry to dropping state
+ * @dropping:		set to true if in dropping state
+ * @first_above_time:	when we went (or will go) continuously above target
+ *			for interval
+ * @drop_next:		time to drop next packet, or when we dropped last
+ * @ldelay:		sojourn time of last dequeued packet
+ */
+struct codel_vars {
+	u32		count;
+	u32		lastcount;
+	bool		dropping;
+	codel_time_t	first_above_time;
+	codel_time_t	drop_next;
+	codel_time_t	ldelay;
+};
+
+/**
+ * struct codel_stats - contains codel shared variables and stats
+ * @maxpacket:	largest packet we've seen so far
+ * @drop_count:	temp count of dropped packets in dequeue()
+ * ecn_mark:	number of packets we ECN marked instead of dropping
+ */
+struct codel_stats {
+	u32		maxpacket;
+	u32		drop_count;
+	u32		ecn_mark;
+};
+
+static void codel_params_init(struct codel_params *params)
+{
+	params->interval = MS2TIME(100);
+	params->target = MS2TIME(5);
+	params->ecn = false;
+}
+
+static void codel_vars_init(struct codel_vars *vars)
+{
+	vars->drop_next = 0;
+	vars->first_above_time = 0;
+	vars->dropping = false; /* exit dropping state */
+	vars->count = 0;
+	vars->lastcount = 0;
+}
+
+static void codel_stats_init(struct codel_stats *stats)
+{
+	stats->maxpacket = 256;
+}
+
+/* return interval/sqrt(x) with good precision
+ * relies on int_sqrt(unsigned long x) kernel implementation
+ */
+static u32 codel_inv_sqrt(u32 _interval, u32 _x)
+{
+	u64 interval = _interval;
+	unsigned long x = _x;
+
+	/* Scale operands for max precision */
+
+#if BITS_PER_LONG == 64
+	x <<= 32; /* On 64bit arches, we can prescale x by 32bits */
+	interval <<= 16;
+#endif
+
+	while (x < (1UL << (BITS_PER_LONG - 2))) {
+		x <<= 2;
+		interval <<= 1;
+	}
+	do_div(interval, int_sqrt(x));
+	return (u32)interval;
+}
+
+static codel_time_t codel_control_law(codel_time_t t,
+				      codel_time_t interval,
+				      u32 count)
+{
+	return t + codel_inv_sqrt(interval, count);
+}
+
+
+static bool codel_should_drop(struct sk_buff *skb,
+			      unsigned int *backlog,
+			      struct codel_vars *vars,
+			      struct codel_params *params,
+			      struct codel_stats *stats,
+			      codel_time_t now)
+{
+	bool ok_to_drop;
+
+	if (!skb) {
+		vars->first_above_time = 0;
+		return false;
+	}
+
+	vars->ldelay = now - codel_get_enqueue_time(skb);
+	*backlog -= qdisc_pkt_len(skb);
+
+	if (unlikely(qdisc_pkt_len(skb) > stats->maxpacket))
+		stats->maxpacket = qdisc_pkt_len(skb);
+
+	if (codel_time_before(vars->ldelay, params->target) ||
+	    *backlog <= stats->maxpacket) { 
+		/* went below - stay below for at least interval */
+		vars->first_above_time = 0;
+		return false;
+	}
+	ok_to_drop = false;
+	if (vars->first_above_time == 0) {
+		/* just went above from below. If we stay above
+		 * for at least interval we'll say it's ok to drop
+		 */
+		vars->first_above_time = now + params->interval;
+	} else if (codel_time_after(now, vars->first_above_time)) {
+		ok_to_drop = true;
+	}
+	return ok_to_drop;
+}
+
+typedef struct sk_buff * (*codel_skb_dequeue_t)(struct codel_vars *vars,
+						struct Qdisc *sch);
+
+static struct sk_buff *codel_dequeue(struct Qdisc *sch,
+				     struct codel_params *params,
+				     struct codel_vars *vars,
+				     struct codel_stats *stats,
+				     codel_skb_dequeue_t dequeue_func,
+				     u32 *backlog)
+{
+	struct sk_buff *skb = dequeue_func(vars, sch);
+	codel_time_t now;
+	bool drop;
+
+	if (!skb) {
+		vars->dropping = false;
+		return skb;
+	}
+	now = codel_get_time();
+	drop = codel_should_drop(skb, backlog, vars, params, stats, now);
+	if (vars->dropping) {
+		if (!drop) {
+			/* sojourn time below target - leave dropping state */
+			vars->dropping = false;
+		} else if (codel_time_after_eq(now, vars->drop_next)) {
+			/* It's time for the next drop. Drop the current
+			 * packet and dequeue the next. The dequeue might 
+			 * take us out of dropping state. 
+			 * If not, schedule the next drop.
+			 * A large backlog might result in drop rates so high
+			 * that the next drop should happen now, 
+			 * hence the while loop.
+			 */  
+			while (vars->dropping && 
+			       codel_time_after_eq(now, vars->drop_next)) {
+				if (++vars->count == 0) /* avoid zero divides */
+					vars->count = ~0U;
+				if (params->ecn && INET_ECN_set_ce(skb)) {
+					stats->ecn_mark++;
+					vars->drop_next =
+						codel_control_law(vars->drop_next,
+								  params->interval,
+								  vars->count);
+					goto end;
+				}
+				qdisc_drop(skb, sch);
+				stats->drop_count++;
+				skb = dequeue_func(vars, sch);
+				if (!codel_should_drop(skb, backlog,
+						       vars, params, stats, now)) {
+					/* leave dropping state */
+					vars->dropping = false;
+				} else {
+					/* and schedule the next drop */
+					vars->drop_next =
+						codel_control_law(vars->drop_next,
+								  params->interval,
+								  vars->count);
+				}
+			}
+		}
+	} else if (drop) {
+		if (params->ecn && INET_ECN_set_ce(skb)) {
+			stats->ecn_mark++;
+		} else {
+			qdisc_drop(skb, sch);
+			stats->drop_count++;
+
+			skb = dequeue_func(vars, sch);
+			drop = codel_should_drop(skb, backlog, vars, params,
+						 stats, now);
+		}
+		vars->dropping = true;
+		/* 
+		 * if min went above target close to when we last went below it
+		 * assume that the drop rate that controlled the queue on the
+		 * last cycle is a good starting point to control it now.
+		 */
+		if (codel_time_before(now - vars->drop_next,
+				      16 * params->interval)) {
+			vars->count = (vars->count - vars->lastcount) | 1;
+		} else {
+			vars->count = 1;
+		}
+		vars->lastcount = vars->count;
+		vars->drop_next = codel_control_law(now, params->interval,
+						    vars->count);
+	}
+end:
+	return skb;
+}
+#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 75b58f8..fadd252 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -250,6 +250,17 @@ config NET_SCH_QFQ
 
 	  If unsure, say N.
 
+config NET_SCH_CODEL
+	tristate "Controlled Delay AQM (CODEL)"
+	help
+	  Say Y here if you want to use the Controlled Delay (CODEL)
+	  packet scheduling algorithm.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called sch_codel.
+
+	  If unsure, say N.
+
 config NET_SCH_INGRESS
 	tristate "Ingress Qdisc"
 	depends on NET_CLS_ACT
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 8cdf4e2..30fab03 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_NET_SCH_PLUG)	+= sch_plug.o
 obj-$(CONFIG_NET_SCH_MQPRIO)	+= sch_mqprio.o
 obj-$(CONFIG_NET_SCH_CHOKE)	+= sch_choke.o
 obj-$(CONFIG_NET_SCH_QFQ)	+= sch_qfq.o
+obj-$(CONFIG_NET_SCH_CODEL)	+= sch_codel.o
 
 obj-$(CONFIG_NET_CLS_U32)	+= cls_u32.o
 obj-$(CONFIG_NET_CLS_ROUTE4)	+= cls_route.o
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
new file mode 100644
index 0000000..a96e95a
--- /dev/null
+++ b/net/sched/sch_codel.c
@@ -0,0 +1,275 @@
+/*
+ * Codel - The Controlled-Delay Active Queue Management algorithm
+ *
+ *  Copyright (C) 2011-2012 Kathleen Nichols <nichols@pollere.com>
+ *  Copyright (C) 2011-2012 Van Jacobson <van@pollere.net>
+ *
+ *  Implemented on linux by :
+ *  Copyright (C) 2012 Michael D. Taht <dave.taht@bufferbloat.net>
+ *  Copyright (C) 2012 Eric Dumazet <edumazet@google.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions, and the following disclaimer,
+ *    without modification.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The names of the authors may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * Alternatively, provided that this notice is retained in full, this
+ * software may be distributed under the terms of the GNU General
+ * Public License ("GPL") version 2, in which case the provisions of the
+ * GPL apply INSTEAD OF those given above.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <net/pkt_sched.h>
+#include <net/codel.h>
+
+
+#define DEFAULT_CODEL_LIMIT 1000
+
+struct codel_sched_data {
+	struct codel_params	params;
+	struct codel_vars	vars;
+	struct codel_stats	stats;
+	u32			drop_overlimit;
+};
+
+/* This is the specific function called from codel_dequeue()
+ * to dequeue a packet from queue. Note: backlog is handled in
+ * codel, we dont need to reduce it here.
+ */
+static struct sk_buff *dequeue(struct codel_vars *vars, struct Qdisc *sch)
+{
+	struct sk_buff *skb = __skb_dequeue(&sch->q);
+
+	prefetch(&skb->end); /* we'll need skb_shinfo() */
+	return skb;
+}
+
+static struct sk_buff *codel_qdisc_dequeue(struct Qdisc *sch)
+{
+	struct codel_sched_data *q = qdisc_priv(sch);
+	struct sk_buff *skb;
+
+	skb = codel_dequeue(sch, &q->params, &q->vars, &q->stats,
+			    dequeue, &sch->qstats.backlog);
+	/* We cant call qdisc_tree_decrease_qlen() if our qlen is 0,
+	 * or HTB crashes. Defer it for next round.
+	 */
+	if (q->stats.drop_count && sch->q.qlen) {
+		qdisc_tree_decrease_qlen(sch, q->stats.drop_count);
+		q->stats.drop_count = 0;
+	}
+	if (skb)
+		qdisc_bstats_update(sch, skb);
+	return skb;
+}
+
+static int codel_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
+{
+	struct codel_sched_data *q;
+
+	if (likely(qdisc_qlen(sch) < sch->limit)) {
+		codel_set_enqueue_time(skb);
+		return qdisc_enqueue_tail(skb, sch);
+	}
+	q = qdisc_priv(sch);
+	q->drop_overlimit++;
+	return qdisc_drop(skb, sch);
+}
+
+static const struct nla_policy codel_policy[TCA_CODEL_MAX + 1] = {
+	[TCA_CODEL_TARGET]	= { .type = NLA_U32 },
+	[TCA_CODEL_LIMIT]	= { .type = NLA_U32 },
+	[TCA_CODEL_INTERVAL]	= { .type = NLA_U32 },
+	[TCA_CODEL_ECN]		= { .type = NLA_U32 },
+};
+
+static int codel_change(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct codel_sched_data *q = qdisc_priv(sch);
+	struct nlattr *tb[TCA_CODEL_MAX + 1];
+	unsigned int qlen;
+	int err;
+
+	if (!opt)
+		return -EINVAL;
+
+	err = nla_parse_nested(tb, TCA_CODEL_MAX, opt, codel_policy);
+	if (err < 0)
+		return err;
+
+	sch_tree_lock(sch);
+
+	if (tb[TCA_CODEL_TARGET]) {
+		u32 target = nla_get_u32(tb[TCA_CODEL_TARGET]);
+
+		q->params.target = ((u64)target * NSEC_PER_USEC) >> CODEL_SHIFT;
+	}
+
+	if (tb[TCA_CODEL_INTERVAL]) {
+		u32 interval = nla_get_u32(tb[TCA_CODEL_INTERVAL]);
+
+		q->params.interval = ((u64)interval * NSEC_PER_USEC) >> CODEL_SHIFT;
+	}
+
+	if (tb[TCA_CODEL_LIMIT])
+		sch->limit = nla_get_u32(tb[TCA_CODEL_LIMIT]);
+
+	if (tb[TCA_CODEL_ECN])
+		q->params.ecn = !!nla_get_u32(tb[TCA_CODEL_ECN]);
+
+	qlen = sch->q.qlen;
+	while (sch->q.qlen > sch->limit) {
+		struct sk_buff *skb = __skb_dequeue(&sch->q);
+
+		sch->qstats.backlog -= qdisc_pkt_len(skb);
+		qdisc_drop(skb, sch);
+	}
+	qdisc_tree_decrease_qlen(sch, qlen - sch->q.qlen);
+
+	sch_tree_unlock(sch);
+	return 0;
+}
+
+static int codel_init(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct codel_sched_data *q = qdisc_priv(sch);
+
+	sch->limit = DEFAULT_CODEL_LIMIT;
+
+	codel_params_init(&q->params);
+	codel_vars_init(&q->vars);
+	codel_stats_init(&q->stats);
+
+	if (opt) {
+		int err = codel_change(sch, opt);
+
+		if (err)
+			return err;
+	}
+
+	if (sch->limit >= 1)
+		sch->flags |= TCQ_F_CAN_BYPASS;
+	else
+		sch->flags &= ~TCQ_F_CAN_BYPASS;
+
+	return 0;
+}
+
+static int codel_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+	struct codel_sched_data *q = qdisc_priv(sch);
+	struct nlattr *opts;
+
+	opts = nla_nest_start(skb, TCA_OPTIONS);
+	if (opts == NULL)
+		goto nla_put_failure;
+
+	if (nla_put_u32(skb, TCA_CODEL_TARGET,
+			codel_time_to_us(q->params.target)) ||
+	    nla_put_u32(skb, TCA_CODEL_LIMIT,
+			sch->limit) ||
+	    nla_put_u32(skb, TCA_CODEL_INTERVAL,
+			codel_time_to_us(q->params.interval)) ||
+	    nla_put_u32(skb, TCA_CODEL_ECN,
+			q->params.ecn))
+		goto nla_put_failure;
+
+	return nla_nest_end(skb, opts);
+
+nla_put_failure:
+	nla_nest_cancel(skb, opts);
+	return -1;
+}
+
+static int codel_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
+{
+	const struct codel_sched_data *q = qdisc_priv(sch);
+	struct tc_codel_xstats st = {
+		.maxpacket	= q->stats.maxpacket,
+		.count		= q->vars.count,
+		.lastcount	= q->vars.lastcount,
+		.drop_overlimit = q->drop_overlimit,
+		.ldelay		= codel_time_to_us(q->vars.ldelay),
+		.dropping	= q->vars.dropping,
+		.ecn_mark	= q->stats.ecn_mark,
+	};
+
+	if (q->vars.dropping) {
+		codel_tdiff_t delta = q->vars.drop_next - codel_get_time();
+		
+		if (delta >= 0)
+			st.drop_next = codel_time_to_us(delta);
+		else
+			st.drop_next = -codel_time_to_us(-delta);
+	}							 
+
+	return gnet_stats_copy_app(d, &st, sizeof(st));
+}
+
+static void codel_reset(struct Qdisc *sch)
+{
+	struct codel_sched_data *q = qdisc_priv(sch);
+
+	qdisc_reset_queue(sch);
+	codel_vars_init(&q->vars);
+}
+
+static struct Qdisc_ops codel_qdisc_ops __read_mostly = {
+	.id		=	"codel",
+	.priv_size	=	sizeof(struct codel_sched_data),
+
+	.enqueue	=	codel_qdisc_enqueue,
+	.dequeue	=	codel_qdisc_dequeue,
+	.peek		=	qdisc_peek_dequeued,
+	.init		=	codel_init,
+	.reset		=	codel_reset,
+	.change 	=	codel_change,
+	.dump		=	codel_dump,
+	.dump_stats	=	codel_dump_stats,
+	.owner		=	THIS_MODULE,
+};
+
+static int __init codel_module_init(void)
+{
+	return register_qdisc(&codel_qdisc_ops);
+}
+
+static void __exit codel_module_exit(void)
+{
+	unregister_qdisc(&codel_qdisc_ops);
+}
+
+module_init(codel_module_init)
+module_exit(codel_module_exit)
+
+MODULE_DESCRIPTION("Controlled Delay queue discipline");
+MODULE_AUTHOR("Dave Taht");
+MODULE_AUTHOR("Eric Dumazet");
+MODULE_LICENSE("Dual BSD/GPL");

^ permalink raw reply related

* Re: [PATCH 0/3] staging: delete duplicated files/functionality in rtl8712
From: Greg Kroah-Hartman @ 2012-05-10 18:11 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	florian.c.schilhabel-gM/Ye1E23mwN+BqQ9rBEUg,
	ali-H4aWS73dXuqdkR0RstYxLQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	Larry Finger
In-Reply-To: <20120510173139.GA16704-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>

On Thu, May 10, 2012 at 01:31:39PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH 0/3] staging: delete duplicated files/functionality in rtl8712] On 10/05/2012 (Thu 09:54) Greg Kroah-Hartman wrote:
> 
> [...]
> 
> > Can you regenerate these with the full deletion content?  There have
> > been some spelling fixes in these files, and your patches do not apply
> > due to them.  I can hand-edit the patches if you regenerate it.
> > 
> > Or if you rebase on the last linux-next tree, that will pick those
> > changes up and then I can apply these properly.
> 
> I've rebased it onto your staging-next of today, so it should be
> hopefully seamless for you now.

Yes, that was even easier, now pulled and pushed out, thanks.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [RFC] net: remove NETIF_F_FRAGLIST from xgmac, ehea, axienet and ll_temac
From: Sebastian Andrzej Siewior @ 2012-05-10 18:20 UTC (permalink / raw)
  To: netdev
  Cc: Thadeu Lima de Souza Cascardo, Andy Gospodarek, Anirudha Sarangi,
	John Linn, Rob Herring, Daniel Borkmann,
	Sebastian Andrzej Siewior

There seem to be two ways how the data payload can be attached to a
sk_buff. That is simple kmalloc() and via "paged" fragments. The former
is simple. The latter attaches each fragment to sh_info->frags and
accounts total number of fragments in sh_info->nr_frags. Once
skb->data_len is non-zero then there should be page fragments available
and sh_info->nr_frags should not be zero anymore.

There is also a third way how data can be added to a skb, that is via
the sh_info->frag_list. This is a complete sk_buff (including data)
which is attached to the former skb. This one can also have linear
memory (kmalloc()) or not (->frags).
If the network driver which should transmit the skb does not support
one of this cool things, then the network core makes a linear skb which
contains only linear kmalloc() memory. If the driver supports the frags
(nr_frags is non zero) then it denotes this by setting the NETIF_F_SG
flag. If the driver is also able to walk via the ->frag_list then it
sets NETIF_F_FRAGLIST flag.

I remove the NETIF_F_FRAGLIST flag here from these drivers here because
I did not find any evidence that they look the ->frag_list at all. It
seems that they look only at sh_info->frags and handle only those pages.
Other drivers which set this flag like ifb don't look at the
->frag_list either but they pass the skb back to the network stack.

The virtio_net driver is using skb_to_sgvec() which gives it a sglist
including everything and tun is using skb_copy_datagram_const_iovec()
which is considering the ->frag_list as well.

So I found evidence that some drivers are doing it right, but for those
five I haven't found any.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/ethernet/calxeda/xgmac.c              |    2 +-
 drivers/net/ethernet/ibm/ehea/ehea_main.c         |    2 +-
 drivers/net/ethernet/tehuti/tehuti.c              |    4 +---
 drivers/net/ethernet/xilinx/ll_temac_main.c       |    2 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c |    2 +-
 drivers/scsi/cxgbi/libcxgbi.c                     |    2 +-
 6 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 11f667f..11fa3a5 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -1772,7 +1772,7 @@ static int xgmac_probe(struct platform_device *pdev)
 	if (device_can_wakeup(priv->device))
 		priv->wolopts = WAKE_MAGIC;	/* Magic Frame as default */
 
-	ndev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA;
+	ndev->hw_features = NETIF_F_SG | NETIF_F_HIGHDMA;
 	if (readl(priv->base + XGMAC_DMA_HW_FEATURE) & DMA_HW_FEAT_TXCOESEL)
 		ndev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 				     NETIF_F_RXCSUM;
diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index c9069a2..2231ad6 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -3029,7 +3029,7 @@ static struct ehea_port *ehea_setup_single_port(struct ehea_adapter *adapter,
 
 	dev->hw_features = NETIF_F_SG | NETIF_F_TSO
 		      | NETIF_F_IP_CSUM | NETIF_F_HW_VLAN_TX | NETIF_F_LRO;
-	dev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO
+	dev->features = NETIF_F_SG | NETIF_F_TSO
 		      | NETIF_F_HIGHDMA | NETIF_F_IP_CSUM | NETIF_F_HW_VLAN_TX
 		      | NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_FILTER
 		      | NETIF_F_RXCSUM;
diff --git a/drivers/net/ethernet/tehuti/tehuti.c b/drivers/net/ethernet/tehuti/tehuti.c
index ad973ff..6c7e519 100644
--- a/drivers/net/ethernet/tehuti/tehuti.c
+++ b/drivers/net/ethernet/tehuti/tehuti.c
@@ -1994,9 +1994,7 @@ bdx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		ndev->irq = pdev->irq;
 		ndev->features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO
 		    | NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX |
-		    NETIF_F_HW_VLAN_FILTER | NETIF_F_RXCSUM
-		    /*| NETIF_F_FRAGLIST */
-		    ;
+		    NETIF_F_HW_VLAN_FILTER | NETIF_F_RXCSUM;
 		ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
 			NETIF_F_TSO | NETIF_F_HW_VLAN_TX;
 
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index d21591a..e804f1f 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -1020,7 +1020,7 @@ static int __devinit temac_of_probe(struct platform_device *op)
 	dev_set_drvdata(&op->dev, ndev);
 	SET_NETDEV_DEV(ndev, &op->dev);
 	ndev->flags &= ~IFF_MULTICAST;  /* clear multicast */
-	ndev->features = NETIF_F_SG | NETIF_F_FRAGLIST;
+	ndev->features = NETIF_F_SG;
 	ndev->netdev_ops = &temac_netdev_ops;
 	ndev->ethtool_ops = &temac_ethtool_ops;
 #if 0
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 9c365e1..fde716d 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1494,7 +1494,7 @@ static int __devinit axienet_of_probe(struct platform_device *op)
 
 	SET_NETDEV_DEV(ndev, &op->dev);
 	ndev->flags &= ~IFF_MULTICAST;  /* clear multicast */
-	ndev->features = NETIF_F_SG | NETIF_F_FRAGLIST;
+	ndev->features = NETIF_F_SG;
 	ndev->netdev_ops = &axienet_netdev_ops;
 	ndev->ethtool_ops = &axienet_ethtool_ops;
 
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index d9253db..7968d4f 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -1968,7 +1968,7 @@ int cxgbi_conn_init_pdu(struct iscsi_task *task, unsigned int offset,
 			}
 			skb_put(skb, count + padlen);
 		} else {
-			/* data fit into frag_list */
+			/* data fit into frags */
 			for (i = 0; i < tdata->nr_frags; i++) {
 				__skb_fill_page_desc(skb, i,
 						tdata->frags[i].page,
-- 
1.7.10

^ permalink raw reply related

* Re: [PATCH RFC 1/6] skbuff: support per-page destructors in copy_ubufs
From: Michael S. Tsirkin @ 2012-05-10 18:42 UTC (permalink / raw)
  To: Ian Campbell; +Cc: David Miller, netdev@vger.kernel.org, eric.dumazet@gmail.com
In-Reply-To: <1336671977.14220.26.camel@zakaz.uk.xensource.com>

On Thu, May 10, 2012 at 06:46:17PM +0100, Ian Campbell wrote:
> On Mon, 2012-05-07 at 14:54 +0100, Michael S. Tsirkin wrote:
> 
> >  	/* skb frags point to kernel buffers */
> >  	for (i = skb_shinfo(skb)->nr_frags; i > 0; i--) {
> > +		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
> 
> This needs to be ....->frags[i - 1]

Good catch.
  	for (i = skb_shinfo(skb)->nr_frags - 1; i >= 0; i--) {
		skb_frag_t *f = &skb_shinfo(skb)->frags[i];

would be a bit clearer though.

> otherwise you put every new frag one too high and don't do anything to
> frag 0, which leaves the old destructor pointer in place and leads to a
> double free.
> 
> I think skb_frag_set_destructor and skb_copy_frag_destructor need to
> clear and propagate respectively (or maybe just clear in both cases) the
> destructor_arg field since it is otherwise not initialised when we set
> SKBTX_DEV_ZEROCOPY and that can trigger wrong behaviour in this
> function.
> 
> Ian.

Agree, let's just clear it.



> > +		if (unlikely((!uarg && !f->page.destructor)))
> > +			continue;
> >  		__skb_fill_page_desc(skb, i-1, head, 0,
> >  				     skb_shinfo(skb)->frags[i - 1].size);
> >  		head = (struct page *)head->private;
> 

So the below on top then. I pushed these on
top of my zerocopy branch - can you confirm pls?

---

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 930a50e..e52bc8d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1270,8 +1270,10 @@ static inline void skb_frag_set_destructor(struct sk_buff *skb, int i,
 {
 	skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 	frag->page.destructor = destroy;
-	if (destroy)
+	if (destroy) {
 		skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+		skb_shinfo(skb)->destructor_arg = NULL;
+	}
 }
 
 /**
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b7fc47e..453f621 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -753,12 +753,11 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 		uarg->callback(uarg);
 
 	/* skb frags point to kernel buffers */
-	for (i = skb_shinfo(skb)->nr_frags; i > 0; i--) {
+	for (i = skb_shinfo(skb)->nr_frags - 1; i >= 0; i--) {
 		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
 		if (unlikely((!uarg && !f->page.destructor)))
 			continue;
-		__skb_fill_page_desc(skb, i-1, head, 0,
-				     skb_shinfo(skb)->frags[i - 1].size);
+		__skb_fill_page_desc(skb, i, head, 0, f->size);
 		head = (struct page *)head->private;
 	}
 

^ permalink raw reply related

* Re: [PATCH] netlink: connector: implement cn_netlink_reply
From: Ben Hutchings @ 2012-05-10 19:22 UTC (permalink / raw)
  To: Alban Crequy
  Cc: Evgeniy Polyakov, netdev, Vincent Sanders,
	Javier Martinez Canillas, Rodrigo Moya
In-Reply-To: <20120510113908.5b79c5f3@chocolatine.cbg.collabora.co.uk>

On Thu, 2012-05-10 at 11:39 +0100, Alban Crequy wrote:
> On Thu, 10 May 2012 04:45:53 +0400,
> Evgeniy Polyakov <zbr@ioremap.net> wrote :
> 
> > On Thu, May 10, 2012 at 01:20:48AM +0100, Ben Hutchings
> > (bhutchings@solarflare.com) wrote:
> > > On Wed, 2012-05-09 at 15:37 +0100, Alban Crequy wrote:
> > > > In a connector callback, it was not possible to reply to a
> > > > message only to a sender. This patch implements
> > > > cn_netlink_reply(). It uses the connector socket to send an
> > > > unicast netlink message back to the sender.
> > > [...]
> > > 
> > > We try to avoid adding functions with no users.  You'll need to
> > > submit the code that's intended to use this as well.
> > 
> > I have no objection against this patch, but as correctly stated it is
> > useless without users. Alban, what is the code you want this
> > functionality to be used in? Do you plan to submit it? Can you submit
> > this change in the patch with your code?
> 
> The code to use the feature is not yet ready for submission and we will
> add this patch to the front of that submission in due course.
> 
> We are just being good community members and making each patch
> available early.

When you send a message with a subject beginning [PATCH] to a kernel
mailing list, it's generally assumed to be a request to the relevant
maintainer to apply that patch.  But kernel API functions are added only
to support features that are exposed exernally, and tend to be removed
when there are no in-tree users.

You could of course send such patches as RFCs ([RFC][PATCH] in the
subject), so it's clear that you don't expect them to be applied yet.

Ben.

> Thanks for your feedback on this patch. Please let
> me know if I can add any reviewed-by.

-- 
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


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