Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] packet: fix broken build.
From: David Miller @ 2012-08-23 16:29 UTC (permalink / raw)
  To: rosenr; +Cc: netdev, xemul, edumazet
In-Reply-To: <309002C0DA137042828828FC53D7A934012B21B8CCCE@IL-MB01.marvell.com>

From: Rami Rosen <rosenr@marvell.com>
Date: Thu, 23 Aug 2012 15:55:41 +0300

> This patch fixes a broken build due to a missing header: 
> ...
>   CC      net/ipv4/proc.o
> In file included from include/net/net_namespace.h:15,
>                  from net/ipv4/proc.c:35:
> include/net/netns/packet.h:11: error: field 'sklist_lock' has incomplete type
> ...
> 
> The lock of netns_packet has been replaced by a recent patch to be a mutex instead of a spinlock, 
> but we need to replace the header file to be linux/mutex.h instead of linux/spinlock.h as well.
> 
> See commit 0fa7fa98dbcc2789409ed24e885485e645803d7f:
> packet: Protect packet sk list with mutex (v2) patch, 
> 
> Signed-off-by: Rami Rosen <rosenr@marvell.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] af_packet: match_fanout_group() can be static
From: David Miller @ 2012-08-23 16:28 UTC (permalink / raw)
  To: fengguang.wu; +Cc: netdev, eric
In-Reply-To: <20120823115121.GA17472@localhost>

From: Fengguang Wu <fengguang.wu@intel.com>
Date: Thu, 23 Aug 2012 19:51:21 +0800

> cc: Eric Leblond <eric@regit.org>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>

Applied, thanks.

^ permalink raw reply

* RE: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
From: Vick, Matthew @ 2012-08-23 16:27 UTC (permalink / raw)
  To: Richard Cochran, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <20120823112837.GE2238@netboy.at.omicron.at>

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Thursday, August 23, 2012 4:29 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
> 
> On Thu, Aug 23, 2012 at 02:56:50AM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> >
> > For users without CONFIG_IGB_PTP=y, we should not be compiling any
> PTP
> > code into the driver. Tidy up the wrapping in igb to support this.
> 
> I would appreciate being put on Cc: for any PTP stuff.
> 
> Also, if you really want to improve the IGB driver, maybe you could
> look at what happens to the time stamping when you do an ifdown/ifup on
> the card. It seems to screw everything (observed on kernel 3.5).
> 
> Thanks,
> Richard

Will do going forward. I appreciate the review.

I actually have two patches coming up that bring some further improvements--these first couple patches are foundation for those. In a patch coming up, the ifdown/ifup issue gets resolved.

Cheers,
Matthew

^ permalink raw reply

* RE: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Vick, Matthew @ 2012-08-23 16:22 UTC (permalink / raw)
  To: Richard Cochran, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <20120823111622.GC2238@netboy.at.omicron.at>

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Thursday, August 23, 2012 4:16 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 11/13] igb: Update PTP function names/variables
> and locations.
> 
> On Thu, Aug 23, 2012 at 02:56:51AM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> >
> > Where possible, move PTP-related functions into igb_ptp.c and update
> > the names of functions and variables to match the established coding
> > style in the files and specify that they are PTP-specific.
> 
> Function renaming as an end in itself? Sounds like churn to me. Also, I
> disagree with some of the displacements.

The goal was to make it clear what was required for the PTP flow and what wasn't. Again, ixgbe was my reference in this. Personally, I disagree with the old naming scheme.

> > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > index 6adb0f7..d1a120e 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > @@ -2280,21 +2280,8 @@ static void igb_get_strings(struct net_device
> *netdev, u32 stringset, u8 *data)
> >  	}
> >  }
> >
> > -static int igb_ethtool_begin(struct net_device *netdev) -{
> > -	struct igb_adapter *adapter = netdev_priv(netdev);
> > -	pm_runtime_get_sync(&adapter->pdev->dev);
> > -	return 0;
> > -}
> > -
> > -static void igb_ethtool_complete(struct net_device *netdev) -{
> > -	struct igb_adapter *adapter = netdev_priv(netdev);
> > -	pm_runtime_put(&adapter->pdev->dev);
> > -}
> > -
> 
> Why have you moved this block? How are these "PTP-related"?

This is just git's diff being silly. :) Rather than move igb_get_ts_info up, git thought I moved those other functions down. I wanted igb_get_ts_info to be with the other get functions.

> >  #ifdef CONFIG_IGB_PTP
> > -static int igb_ethtool_get_ts_info(struct net_device *dev,
> > +static int igb_get_ts_info(struct net_device *dev,
> 
> I like the old name better.

The old name is out of the coding style of igb. Every other function is igb_get_* or igb_set_*, with the exception of igb_ethtool_begin and igb_ethtool_complete. Are you suggesting we add _ethtool to every ethtool function in igb? 

> > -#ifdef CONFIG_IGB_PTP
> > -/**
> > - * igb_tx_hwtstamp - utility function which checks for TX time stamp
> > - * @q_vector: pointer to q_vector containing needed info
> > - * @buffer: pointer to igb_tx_buffer structure
> > - *
> > - * If we were asked to do hardware stamping and such a time stamp is
> > - * available, then it must have been for this skb here because we
> > only
> > - * allow only one such packet into the queue.
> > - */
> > -static void igb_tx_hwtstamp(struct igb_q_vector *q_vector,
> > -			    struct igb_tx_buffer *buffer_info)
> > -{
> > -	struct igb_adapter *adapter = q_vector->adapter;
> > -	struct e1000_hw *hw = &adapter->hw;
> > -	struct skb_shared_hwtstamps shhwtstamps;
> > -	u64 regval;
> > -
> > -	/* if skb does not support hw timestamp or TX stamp not valid
> exit */
> > -	if (likely(!(buffer_info->tx_flags & IGB_TX_FLAGS_TSTAMP)) ||
> > -	    !(rd32(E1000_TSYNCTXCTL) & E1000_TSYNCTXCTL_VALID))
> > -		return;
> > -
> > -	regval = rd32(E1000_TXSTMPL);
> > -	regval |= (u64)rd32(E1000_TXSTMPH) << 32;
> > -
> > -	igb_systim_to_hwtstamp(adapter, &shhwtstamps, regval);
> > -	skb_tstamp_tx(buffer_info->skb, &shhwtstamps);
> > -}
> > -#endif /* CONFIG_IGB_PTP */
> > -
> 
> Here you have taken a static local function and made it into a global
> function. This can have performance impacts.

Which, this function calls igb_systim_to_hwtstamp anyway, which is global. Also, in a follow-on patch I have coming, igb_ptp_tx_hwtstamp won't even be called in clean_tx_irq, FWIW.

> >  /**
> >   * igb_clean_tx_irq - Reclaim resources after transmit completes
> >   * @q_vector: pointer to q_vector containing needed info @@ -5827,7
> > +5796,7 @@ static bool igb_clean_tx_irq(struct igb_q_vector
> *q_vector)
> >
> >  #ifdef CONFIG_IGB_PTP
> >  		/* retrieve hardware timestamp */
> > -		igb_tx_hwtstamp(q_vector, tx_buffer);
> > +		igb_ptp_tx_hwtstamp(q_vector, tx_buffer);
> 
> This name stinks, too. You know that you can have time stamping all by
> itself, right? It is logically separate from the ptp clock stuff.
> 
> This patch doesn't really improve the driver at all, IMHO.
> 
> Thanks,
> Richard

Yes, I'm aware. But, as it stands today, we don't use it for anything else. If the function is feature specific, then we should be calling it out as such.

I'm sorry you feel like this patch doesn't improve the driver. The goal is code cleanup and consistency, both of which I consider to be driver improvements and is why I made the patches. 

Cheers,
Matthew

^ permalink raw reply

* Re: [PATCH net-next] net: reinstate rtnl in call_netdevice_notifiers()
From: David Miller @ 2012-08-23 16:24 UTC (permalink / raw)
  To: ebiederm; +Cc: eric.dumazet, gaofeng, netdev, therbert, maheshb
In-Reply-To: <87harum39o.fsf@xmission.com>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 23 Aug 2012 01:16:51 -0700

> Eric Dumazet <eric.dumazet@gmail.com> writes:
> 
>> From: Eric Dumazet <edumazet@google.com>
>>
>> Eric Biederman pointed out that not holding RTNL while calling
>> call_netdevice_notifiers() was racy.
>>
>> This patch is a direct transcription his feedback
>> against commit 0115e8e30d6fc (net: remove delay at device dismantle)
>>
>> Thanks Eric !
> 
> Looks good to me.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH RFC 0/2] Interface for TCP Metrics
From: Julian Anastasov @ 2012-08-23 16:26 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20120823082235.27c057f6@nehalam.linuxnetplumber.net>


	Hello,

On Thu, 23 Aug 2012, Stephen Hemminger wrote:

> Some of netlink API comments:
> 
> 1. My preference is to not make it so granular.
>    I.e make things like RTT a structure with value and variance
>    rather than lots of independent objects.

	Even the ip route used nested objects for metrics.
I preferred not to add fixed structure for them if that is
what you mean. I implemented it in the same way, just like
rtnetlink_put_metrics().

> 2. Make sure netlink API is symmetric. Get should report all
>    values and Set should accept the same values (but ignore
>    if value can not be modified).

	Will take care if one day we support SET request.

> 3. Try and make ip command output invertable, as in what you
>    print is close to what ip command arguments are.

	Any example for what you mean?

> 4. What about monitoring changes?

	I preferred to avoid them because if the routes
are small number and can be monitored, these entries are
modified very often and to me it looked overkill to generate
notification for every update just to notice that nobody
listens for it.

	BTW, I noticed that the GENL part in iproute is
not separated in common place. Should we create common
place for functions like genl_ctrl_resolve_family and
genl_parse_getfamily ?

	Thanks for the comments!

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [PATCH RFC 1/2] tcp: add generic netlink support for tcp_metrics
From: Eric Dumazet @ 2012-08-23 16:19 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: netdev
In-Reply-To: <1345734105-28328-2-git-send-email-ja@ssi.bg>

On Thu, 2012-08-23 at 18:01 +0300, Julian Anastasov wrote:
> 	Add support for genl "tcp_metrics". No locking
> is changed, only that now we can unlink and delete
> entries after grace period. We implement get/del for
> single entry and dump to support show/flush filtering
> in user space.
> 

Very nice, thanks !

> +static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct tcpm_hash_bucket *hb;
> +	struct tcp_metrics_block *tm;
> +	struct tcp_metrics_block __rcu **pp;
> +	struct inetpeer_addr addr;
> +	unsigned int hash;
> +	struct net *net = genl_info_net(info);
> +	int ret;
> +
> +	ret = parse_nl_addr(info, &addr, &hash, 1);
> +	if (ret < 0)
> +		return ret;
> +	/* Flush all ? */
> +	if (ret > 0) {
> +		unsigned int max_rows = 1U << net->ipv4.tcp_metrics_hash_log;
> +		unsigned int sync_count = 0;
> +		unsigned int row;
> +
> +		hb = net->ipv4.tcp_metrics_hash;
> +		for (row = 0; row < max_rows; row++, hb++) {
> +			spin_lock_bh(&tcp_metrics_lock);
> +			tm = deref_locked_genl(hb->chain);
> +			if (tm)
> +				rcu_assign_pointer(hb->chain, NULL);
> +			spin_unlock_bh(&tcp_metrics_lock);
> +			while (tm) {
> +				struct tcp_metrics_block *next;
> +
> +				next = deref_genl(tm->tcpm_next);
> +				kfree_rcu(tm, rcu_head);
> +				if (!((++sync_count) & 2047))
> +					synchronize_rcu();
> +				tm = next;
> +			}
> +		}
> +		return 0;
> +	}
> +
> +	hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
> +	hb = net->ipv4.tcp_metrics_hash + hash;
> +	pp = &hb->chain;
> +	spin_lock_bh(&tcp_metrics_lock);
> +	for (tm = deref_locked_genl(*pp); tm;
> +	     pp = &tm->tcpm_next, tm = deref_locked_genl(*pp)) {
> +		if (addr_same(&tm->tcpm_addr, &addr)) {
> +			rcu_assign_pointer(*pp, tm->tcpm_next);

Hmm, try "make C=2 net/ipv4/tcp_metrics.o"

with :

CONFIG_SPARSE_RCU_POINTER=y


I guess you can use plain " *pp = tm->tcpm_next;"

> +			break;
> +		}
> +	}
> +	spin_unlock_bh(&tcp_metrics_lock);
> +	if (!tm)
> +		return -ESRCH;
> +	kfree_rcu(tm, rcu_head);
> +	return 0;
> +}
> +

Could you split this in two functions, adding tcp_metrics_flush_all() ?

^ permalink raw reply

* Re: [PATCH 1/2] ipv6: do not hold route table lock when send ndisc probe
From: Eric Dumazet @ 2012-08-23 16:10 UTC (permalink / raw)
  To: Cong Wang
  Cc: Debabrata Banerjee, netdev, Banerjee, Debabrata, David S. Miller,
	Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1345520643.12468.6.camel@cr0>

On Tue, 2012-08-21 at 11:44 +0800, Cong Wang wrote:
> Hi, Debabrata,
> 
> Could you help to test the attached patch below?
> 
> Thanks!
> 

Hard to comment on your patch since its not inlined.

+               nw = kmalloc(sizeof(*nw), GFP_ATOMIC);
+               if (nw) {
+                       memcpy(&nw->target, &neigh->primary_key, sizeof(struct in6_addr));
+                       addrconf_addr_solict_mult(&nw->target, &nw->mcaddr);
+                       nw->dev = rt->dst.dev;
+                       INIT_WORK(&nw->work, queue_ndisc);
+                       schedule_work(&nw->work);
+               }

You cant do that without taking extra reference on dev,
and release it in queue_ndisc()

This also will add interesting side effects at device dismantle.

^ permalink raw reply

* RE: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
From: Vick, Matthew @ 2012-08-23 16:09 UTC (permalink / raw)
  To: Richard Cochran, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <20120823110357.GB2238@netboy.at.omicron.at>

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Thursday, August 23, 2012 4:04 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
> 
> On Thu, Aug 23, 2012 at 02:56:50AM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> >
> > For users without CONFIG_IGB_PTP=y, we should not be compiling any
> PTP
> > code into the driver. Tidy up the wrapping in igb to support this.
> 
> Actually, you are doing more than that. You are adding a bunch of
> comments onto the already existing #endifs.

Fair enough. Would you like me to update the patch description?

> > +#ifdef CONFIG_IGB_PTP
> >  	struct ptp_clock *ptp_clock;
> >  	struct ptp_clock_info caps;
> >  	struct delayed_work overflow_work;
> >  	spinlock_t tmreg_lock;
> >  	struct cyclecounter cc;
> >  	struct timecounter tc;
> > +#endif /* CONFIG_IGB_PTP */
> > +
> 
> This is legitimate, to reduce memory footprint.
> 
> [...]
> 
> But this is just churn. You aren't actually improving anything here. It
> is already clear that the #endif belongs to the #ifdef just three lines
> above.
> 
> If you are on some kind of campaign to comment all the endifs, then you
> should say so, and, to be consistent, you should not overlook the
> CONFIG_IGB_DCA blocks.
> 
> But I think it really isn't needed.
> 
> Thanks,
> Richard

I'm willing to work on a follow on patch to add proper endings to CONFIG_IGB_DCA blocks. The reasoning here was "I'm working on the wrapping for the code here anyway, so I might as well make it consistent."

Personally, I think it is an improvement. If you want to add #endif comments, you may as well be consistent about it.

Cheers,
Matthew

^ permalink raw reply

* Re: [PATCH 2/3] ipvs: Fix faulty IPv6 extension header handling in IPVS
From: Julian Anastasov @ 2012-08-23 16:06 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: netdev, Patrick McHardy, Hans Schillstrom, lvs-devel,
	Simon Horman, Wensong Zhang, netfilter-devel
In-Reply-To: <1345726251.3069.369.camel@localhost>


	Hello,

On Thu, 23 Aug 2012, Jesper Dangaard Brouer wrote:

> > > +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> > > +#include <linux/netfilter_ipv6/ip6_tables.h>
> > > +#endif
> > 
> > 	There is already #if IS_ENABLED(CONFIG_IPV6) that
> > can replace #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> 
> Okay.
> 
> > 	It seems we need IS_ENABLED for many places:
> > CONFIG_NF_CONNTRACK, CONFIG_NF_DEFRAG_IPV6
> 
> Wondering if we should keep these cleanup changes to a separate patch.

	The way you prefer. We can fix it after this
patchset.

> > >  	/* Now find the contained IP header */
> > > -	offset += sizeof(_icmph);
> > > -	cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
> > > -	if (cih == NULL)
> > > -		return NF_ACCEPT; /* The packet looks wrong, ignore */
> > > +	ciph.len = iph->len + sizeof(_icmph);
> > > +	ciph.flags = 0;
> > > +	ciph.fragoffs = 0;
> > > +	ciph.protocol = ipv6_find_hdr(skb, &ciph.len, -1, &ciph.fragoffs,
> > > +				      &ciph.flags);
> 
> (notice that &ciph.len can get updated by ipv6_find_hdr())
> 
> > > +	ciph.saddr = iph->saddr;	/* con_in_get() handles reverse order */
> > > +	ciph.daddr = iph->daddr;
> > 
> > 	The ciph initialization looks dangerous if one day
> > we add new field into the header.
> > 
> > 	Can we use ciph = (struct ip_vs_iphdr) { .XXX = val, ... },
> > in such case we have to call ipv6_find_hdr out of (after)
> > this initialization? Of course, we will write twice to
> > small fields such as protocol, len, fragoffs, flags
> 
> I'm not sure I follow/understand.

	For example:

	ciph = (struct ip_vs_iphdr) {
		.len = iph->len + sizeof(_icmph),
		.saddr = iph->saddr,
		.daddr = iph->daddr,
	};
	ciph.protocol = ipv6_find_hdr(skb, &ciph.len, -1, &ciph.fragoffs,
				      &ciph.flags);

	but I'm not sure if the saddr/daddr part compiles.

> > 	Also ipv6_find_hdr looks a bit noisy for missing header,
> > can it be a problem for the inner IPv6 header in ICMP messages?
> 
> I can see, that I don't handle the error cases of missing headers, from
> a call to ipv6_find_hdr() ... I guess I need to check if the return
> value "protocol" is negative.  But I'm not sure if it matters in this
> case (Hans?)

	ok. But I was referring to this message:

printk(KERN_ERR "IPv6 header not found\n");

> > 	In patch 3 ip_vs_in_icmp_v6 initializes ciph in the
> > same way. It will be difficult to audit the code later
> > considering the large number of places where iph is used.
> 
> I'm not sure what you want me to do?

	May be it is only in ip_vs_in_icmp_v6, my above
example is for ip_vs_in_icmp_v6, let me know if it
compiles.

> > 	Still in the same func, above code is correct but
> > patch 3 changes it back to wrong state (offs_ciph = ciph.len).
> 
> Don't think its a "bug" in patch3, it might be a "bug" in this patch.
> Because &ciph.len gets updated earlier by ipv6_find_hdr()... but I'm
> starting to get confused.
> Perhaps we should move these changes to ip_vs_in_icmp_v6() into the same
> patch?

	ip_vs_icmp_xmit_v6 expects to see length that
will be made writeable, not exactly offset of header.
May be you can rename the ip_vs_icmp_xmit_v6 argument
from offset to wrtlen, it covers the ports because we
are going to mangle them. Patch 3 should not change
anymore there.

> > 	In this context, may be soon we will modify ip_vs_ftp to
> > support IPv6. It is possible to work with present FTP helper
> > in netfilter. Does it mean that we will see only reassembled
> > packets when conntrack is running? No original fragments.
> 
> Yes, it seems that we will *only* see the reassembled packet (no
> original fragments) after Patricks patches.  BUT *only* when loading the
> module nf_conntrack_ipv6.

	Yes, ok

> > Are we prepared to work in both ways (originals+reasm and
> > just reasm) ?
> 
> As mentioned in another thread, no. But only because the reassembled
> packet will be dropped due to the MTU check. After this is fixed, the
> ipvs code seems to work. (Notice, this is both without and with my/these
> patches)

	Very good

> > - For patch 3 in Kconfig do we need 'select NF_DEFRAG_IPV6' ?
> 
> Yes, but it is still possible not to load the module nf_defrag_ipv6.

	nf_defrag_ipv6 becomes mandatory for IPVS-IPv6.
Conntrack should be optional.

> When not loading, nf_defrag_ipv6, these patches have no effect, and no
> fragments are passed through.

	What happens with fragments if nf_defrag_ipv6 is
not loaded? IPVS will see original fragments without
reasm ptr? I assume IPVS can see them but can not do
much except to mangle addresses and ports.

> > 	Basicly, I'm concerned what will happen when we
> > start to mangle the protocol payloads (FTP). For now
> > we are safe by touching only addresses and ports. May be
> > we have to synchronize these changes with the work from
> > Patrick.
> 
> Yes, I think we should take Patrick's work into account.
> 
> My biggest concern is, that depending on which modules (nf_defrag_ipv6
> only, or also nf_conntrack_ipv6) are loaded, different code paths are
> used (to support IPv6 fragments for IPVS).

	May be just like for IPv4, I think, nf_defrag_ipv6
should be loaded (added as dep) but nf_conntrack_ipv6 should
be required only for FTP as we use some code from there.

> This will be hard to understand, from a user perspective.  And also
> difficult for us, when users report bugs...
> 
> Is loading nf_conntrack_ipv6 considered a big performance problem/issue
> for IPVS?
> (Can we tell people, to enable conntrack for frag support?)

	We should avoid it if possible. There can be small
routers that do not want conntrack. It would be better if
IPVS uses some symbol that will cause nf_defrag_ipv6 to
load, now we can add it as dep but what if user does not
load it? nf_defrag_ipv6_hooks.c is very small, I'm not
sure if we have to duplicate it just to cause defrag at
prerouting or even input to work for us in the case without
conntrack. Without this support we can not mangle FTP
payload.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* RE: [net-next 13/13] igb: Store the MAC address in the name in the PTP struct.
From: Vick, Matthew @ 2012-08-23 16:05 UTC (permalink / raw)
  To: Richard Cochran, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <20120823104554.GA2238@netboy.at.omicron.at>

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Thursday, August 23, 2012 3:46 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 13/13] igb: Store the MAC address in the name in
> the PTP struct.
> 
> On Thu, Aug 23, 2012 at 02:56:53AM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> >
> > Change the name of the adapter in the PTP struct to enable easier
> > correlation between interface and PTP device.
> 
> You want to put the MAC address into the clock name? This is wrong.
> 
> Besides, there is no need for this. The ethool method already makes the
> correlation crystal clear.
> 
> Thanks,
> Richard

Actually, the name today is wrong. "igb-82580" is inapplicable to I350, I210, and I211 devices. This patch aims to bring some sense to the naming while fixing it. ixgbe implements the same naming scheme.

Cheers,
Matthew

^ permalink raw reply

* Re: IPv6 deadlock with CONFIG_IPV6_ROUTER_PREF
From: Banerjee, Debabrata @ 2012-08-23 15:51 UTC (permalink / raw)
  To: Lin Ming
  Cc: netdev@vger.kernel.org, Hunt, Joshua, dbavatar@gmail.com,
	Lubashev, Igor, linux-kernel@vger.kernel.org
In-Reply-To: <CAF1ivSZN8PyPhAkyaFL-GsTbnr4uwP=zzzjZbgqOKts_wgngYA@mail.gmail.com>

On Aug 23, 2012, at 11:11 AM, "Lin Ming" <mlin@ss.pku.edu.cn> wrote:

> On Fri, Aug 17, 2012 at 1:58 AM, Banerjee, Debabrata
> <dbanerje@akamai.com> wrote:
>> This code tries to send a neighbor discovery ICMPv6 packet for router
>> reachability while read_lock(tb6_lock) is held. The send may want to cause
>> a fib6_clean_all() garbage collection, which will try to take
>> write_lock(tb6_lock), resulting in deadlock. Garbage collection becomes
>> more likely under high load of cloned routes, so this is exploitable as a
>> DDOS attack, given enough attack hosts in relation max_size of the route
>> table (default of 4k). I checked from 3.6-rc1 back to 2.6.32, it is
>> present everywhere.
> 
> How about moving the garbage collection to a kernel thread?
> Then the write_lock(tb6_lock) in this kernel thread won't cause such
> kind of dead lock with other threads.
> 
> Lin Ming

It already runs in a thread, this is a forced gc because we are over the gc threshold, and this thread deadlocks with itself. I thing Cong Wang is on the right track with his patch. The implementation of this RFC must be fixed.

-Debabrata

^ permalink raw reply

* Re: wireless router performance differ with 2.4Ghz vs 5.0Ghz frequency
From: Chris Friesen @ 2012-08-23 15:23 UTC (permalink / raw)
  To: Julian Calaby; +Cc: Lin Ming, linux-wireless, networking
In-Reply-To: <CAGRGNgWddAH3UyYq5T7iT+FuX8ajdGpP6rfjjCUkZZpiCoTmpQ@mail.gmail.com>

On 08/23/2012 04:34 AM, Julian Calaby wrote:
> Hi,
>
> On Thu, Aug 23, 2012 at 2:50 PM, Lin Ming<mlin@ss.pku.edu.cn>  wrote:
>> Hi list,
>>
>> I have a wireless router with 2 wireless cards: 2.4Ghz and 5.0Ghz frequency.
>>
>> The performance for 2.4Ghz is much worse than 5.0Ghz.
>>
>> Does the frequency affect performance so much?
> There are a lot of factors:
> - Amounts of other networks on the same channel
> - Noise
> - Signal strength
> etc.
>
> It's quite possible that there are less networks on 5GHz channels
> where you are so there's less noise, stronger signal, and therefore
> better performance. Most routers are still only 2.4GHz.

In addition to this you may wish to check your channel width.  You may 
be using 40MHz channels on 5GHz and 20MHz channels on 2.4GHz.

Chris

^ permalink raw reply

* Re: [PATCH RFC 0/2] Interface for TCP Metrics
From: Stephen Hemminger @ 2012-08-23 15:22 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: netdev
In-Reply-To: <1345734105-28328-1-git-send-email-ja@ssi.bg>

On Thu, 23 Aug 2012 18:01:43 +0300
Julian Anastasov <ja@ssi.bg> wrote:

> 	This patchset contains 2 patches, one for kernel
> and one for iproute2. We add DUMP/GET/DEL support for
> genl "tcp_metrics" and minimal support for filtering
> by address prefix and family in user space.
> 
> 	I tested show/del/flush, filtering by family, prefix,
> output for metrics such as rtt, rttvar, cwnd, ssthresh (after
> modifying route). I didn't tested output for fast open.
> 
> 	May be some corrections in output can be desired.
> 
> 	The kernel patch has some parts to check:
> 
> - in tcp_metrics_nl_cmd_del I'm trying to flush all addresses
> with single request, eg. when no family, address or other
> selectors are provided. I use some arbitrary counter sync_count
> to force memory to be freed in parts by using synchronize_rcu,
> note that we are under genl_mutex, so may be this is bad idea
> to delay other genl users? My idea was to avoid many commands
> on "flush all", may be we should trigger flush by using some
> other mechanism that is pernet? And I don't know how to test
> it properly without large cache.
> 
> - This function now uses rcu_dereference_protected, someone
> should check if these lockdep checks are correct. I enabled
> lockdep in config and don't see any warnings.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Some of netlink API comments:

1. My preference is to not make it so granular.
   I.e make things like RTT a structure with value and variance
   rather than lots of independent objects.

2. Make sure netlink API is symmetric. Get should report all
   values and Set should accept the same values (but ignore
   if value can not be modified).

3. Try and make ip command output invertable, as in what you
   print is close to what ip command arguments are.

4. What about monitoring changes?

^ permalink raw reply

* Re: IPv6 deadlock with CONFIG_IPV6_ROUTER_PREF
From: Lin Ming @ 2012-08-23 15:10 UTC (permalink / raw)
  To: Banerjee, Debabrata
  Cc: netdev@vger.kernel.org, Hunt, Joshua, dbavatar@gmail.com,
	Lubashev, Igor, linux-kernel@vger.kernel.org
In-Reply-To: <CC52AD11.7CC2%dbanerje@akamai.com>

On Fri, Aug 17, 2012 at 1:58 AM, Banerjee, Debabrata
<dbanerje@akamai.com> wrote:
> This code tries to send a neighbor discovery ICMPv6 packet for router
> reachability while read_lock(tb6_lock) is held. The send may want to cause
> a fib6_clean_all() garbage collection, which will try to take
> write_lock(tb6_lock), resulting in deadlock. Garbage collection becomes
> more likely under high load of cloned routes, so this is exploitable as a
> DDOS attack, given enough attack hosts in relation max_size of the route
> table (default of 4k). I checked from 3.6-rc1 back to 2.6.32, it is
> present everywhere.

How about moving the garbage collection to a kernel thread?
Then the write_lock(tb6_lock) in this kernel thread won't cause such
kind of dead lock with other threads.

Lin Ming

>
> Stack trace below.
>
> Thanks,
> Debabrata
>
> [46476.055009] Pid: 7963, comm: xxxx Not tainted 2.6.38-amd64
> [46476.055009] RIP: 0010:[<ffffffff812878c9>]  [<ffffffff812878c9>]
> __write_lock_failed+0x9/0x20
> [46476.055009] RSP: 0018:ffff8801a099f8f0  EFLAGS: 00200287
> [46476.055009] RAX: ffff8801a099ffd8 RBX: 0000000000000000 RCX:
> 0000000000000000
> [46476.055009] RDX: 0000000000000000 RSI: ffffffffa0196e60 RDI:
> ffff88020bc95454
> [46476.055009] RBP: ffff8801a099f908 R08: ffff8801a099fb78 R09:
> 0000000000000003
> [46476.055009] R10: ffff8801a099fa38 R11: ffff88020ebf1c00 R12:
> ffffffff8100370e
> [46476.055009] R13: 0000000000000000 R14: 0000000000000000 R15:
> 0000000000000000
> [46476.055009] FS:  00007fa1f4a596d0(0000) GS:ffff8800e7c00000(0063)
> knlGS:00000000f6a5fba0
> [46476.055009] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
> [46476.055009] CR2: 00000000f7791000 CR3: 00000001a0bcc000 CR4:
> 00000000000006f0
> [46476.055009] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [46476.055009] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [46476.055009] Process xxxx (pid: 7963, threadinfo ffff8801a099e000, task
> ffff8801a099c880)
> [46476.055009] Stack:
> [46476.055009]  ffffffff81482c17 ffff8801a099f928 ffff88020bc95454
> ffff8801a099f948
> [46476.055009]  ffffffffa01972f9 ffffffffa0196e60 0000000000000200
> ffffffff81960a80
> [46476.055009]  0000000000000d80 000000000000ea60 00000001004cbccc
> ffff8801a099f968
> [46476.055009] Call Trace:
> [46476.055009]  [<ffffffff81482c17>] ? _raw_write_lock_bh+0x27/0x30
> (deadlock on write_lock tb6_lock)
> [46476.055009]  [<ffffffffa01972f9>] fib6_clean_all+0x49/0x90 [ipv6]
> [46476.055009]  [<ffffffffa0196e60>] ? fib6_age+0x0/0x80 [ipv6]
> [46476.055009]  [<ffffffffa019744f>] fib6_run_gc+0x4f/0xe0 [ipv6]
> [46476.055009]  [<ffffffffa0193547>] ip6_dst_gc+0x97/0x120 [ipv6]
> [46476.055009]  [<ffffffff813d5515>] dst_alloc+0xa5/0xc0
> [46476.055009]  [<ffffffffa0196c91>] icmp6_dst_alloc+0x51/0x170 [ipv6]
> [46476.055009]  [<ffffffffa019ac3f>] ndisc_send_skb+0x6f/0x2c0 [ipv6]
> [46476.055009]  [<ffffffff81481b2d>] ?
> schedule_hrtimeout_range_clock+0xcd/0x110
> [46476.055009]  [<ffffffffa019aef1>] __ndisc_send+0x61/0x80 [ipv6]
> [46476.055009]  [<ffffffffa019afbc>] ndisc_send_ns+0x6c/0xa0 [ipv6]
> [46476.055009]  [<ffffffffa0195459>] rt6_probe+0xc9/0xd0 [ipv6]
> [46476.055009]  [<ffffffff81120e50>] ? __pollwait+0x0/0x100
> [46476.055009]  [<ffffffffa0195575>] find_match+0x115/0x180 [ipv6]
> [46476.055009]  [<ffffffffa01956b3>] ip6_pol_route+0xd3/0x2d0 [ipv6]
> (read_lock tb6_lock)
> [46476.055009]  [<ffffffffa01958c6>] ip6_pol_route_output+0x16/0x20 [ipv6]
> [46476.055009]  [<ffffffffa0196dfe>] fib6_rule_lookup+0x1e/0x20 [ipv6]
> [46476.055009]  [<ffffffffa01948c1>] ip6_route_output+0x61/0xa0 [ipv6]
> [46476.055009]  [<ffffffffa0188232>] ip6_dst_lookup_tail+0xe2/0xf0 [ipv6]
> [46476.055009]  [<ffffffffa0188255>] ip6_dst_lookup+0x15/0x20 [ipv6]
> [46476.055009]  [<ffffffffa01aca8c>] tcp_v6_connect+0x26c/0x6e0 [ipv6]
> [46476.055009]  [<ffffffff81235a36>] ? security_sk_alloc+0x16/0x20
> [46476.055009]  [<ffffffff8142be49>] inet_stream_connect+0x2a9/0x300
> [46476.055009]  [<ffffffff81482be4>] ? _raw_spin_unlock_bh+0x14/0x20
> [46476.055009]  [<ffffffff813be329>] ? release_sock+0xd9/0x110
> [46476.055009]  [<ffffffff813bc00f>] sys_connect+0xaf/0xd0
> [46476.055009]  [<ffffffff813e4077>] ? compat_sys_setsockopt+0x87/0x220
> [46476.055009]  [<ffffffff81150e28>] ? compat_sys_fcntl64+0x1d8/0x380
> [46476.055009]  [<ffffffff813e4c93>] compat_sys_socketcall+0x93/0x1f0
> [46476.055009]  [<ffffffff810354ec>] cstar_dispatch+0x7/0x32
> [46476.055009] Code: 00 00 48 8b 5b 20 48 83 eb 07 48 39 d9 73 06 48 89 01
> 31 c0 c3 b8 f2 ff ff ff c3 90 90 90 90 90 90 90 f0 81 07 00 00 00 01 f3 90
> <81> 3f 00 00 00 01 75 f6 f0 81 2f 00 00 00 01 0f 85 e2 ff ff ff
>
> --
> 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/

^ permalink raw reply

* [PATCH RFC 2/2] iproute2: add support for tcp_metrics
From: Julian Anastasov @ 2012-08-23 15:01 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1345734105-28328-1-git-send-email-ja@ssi.bg>

	ip tcp_metrics/tcpmetrics

diff -urpN iproute2-3.5.1/include/linux/tcp_metrics.h iproute2-3.5.1-tcp_metrics/include/linux/tcp_metrics.h
--- iproute2-3.5.1/include/linux/tcp_metrics.h	1970-01-01 02:00:00.000000000 +0200
+++ iproute2-3.5.1-tcp_metrics/include/linux/tcp_metrics.h	2012-08-23 09:50:54.385569009 +0300
@@ -0,0 +1,54 @@
+/* tcp_metrics.h - TCP Metrics Interface */
+
+#ifndef _LINUX_TCP_METRICS_H
+#define _LINUX_TCP_METRICS_H
+
+#include <linux/types.h>
+
+/* NETLINK_GENERIC related info
+ */
+#define TCP_METRICS_GENL_NAME		"tcp_metrics"
+#define TCP_METRICS_GENL_VERSION	0x1
+
+enum tcp_metric_index {
+	TCP_METRIC_RTT,
+	TCP_METRIC_RTTVAR,
+	TCP_METRIC_SSTHRESH,
+	TCP_METRIC_CWND,
+	TCP_METRIC_REORDERING,
+
+	/* Always last.  */
+	__TCP_METRIC_MAX,
+};
+
+#define TCP_METRIC_MAX	(__TCP_METRIC_MAX - 1)
+
+enum {
+	TCP_METRICS_ATTR_UNSPEC,
+	TCP_METRICS_ATTR_ADDR_IPV4,		/* u32 */
+	TCP_METRICS_ATTR_ADDR_IPV6,		/* binary */
+	TCP_METRICS_ATTR_AGE,			/* msecs */
+	TCP_METRICS_ATTR_TW_TSVAL,		/* u32, raw, rcv tsval */
+	TCP_METRICS_ATTR_TW_TS_STAMP,		/* s32, sec age */
+	TCP_METRICS_ATTR_VALS,			/* nested +1, u32 */
+	TCP_METRICS_ATTR_FOPEN_MSS,		/* u16 */
+	TCP_METRICS_ATTR_FOPEN_SYN_DROPS,	/* u16, count of drops */
+	TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS,	/* msecs age */
+	TCP_METRICS_ATTR_FOPEN_COOKIE,		/* binary */
+
+	__TCP_METRICS_ATTR_MAX,
+};
+
+#define TCP_METRICS_ATTR_MAX	(__TCP_METRICS_ATTR_MAX - 1)
+
+enum {
+	TCP_METRICS_CMD_UNSPEC,
+	TCP_METRICS_CMD_GET,
+	TCP_METRICS_CMD_DEL,
+
+	__TCP_METRICS_CMD_MAX,
+};
+
+#define TCP_METRICS_CMD_MAX	(__TCP_METRICS_CMD_MAX - 1)
+
+#endif /* _LINUX_TCP_METRICS_H */
diff -urpN iproute2-3.5.1/ip/Makefile iproute2-3.5.1-tcp_metrics/ip/Makefile
--- iproute2-3.5.1/ip/Makefile	2012-08-13 18:13:58.000000000 +0300
+++ iproute2-3.5.1-tcp_metrics/ip/Makefile	2012-08-23 10:24:01.561660891 +0300
@@ -3,7 +3,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o ipr
     ipmaddr.o ipmonitor.o ipmroute.o ipprefix.o iptuntap.o \
     ipxfrm.o xfrm_state.o xfrm_policy.o xfrm_monitor.o \
     iplink_vlan.o link_veth.o link_gre.o iplink_can.o \
-    iplink_macvlan.o iplink_macvtap.o ipl2tp.o
+    iplink_macvlan.o iplink_macvtap.o ipl2tp.o tcp_metrics.o
 
 RTMONOBJ=rtmon.o
 
diff -urpN iproute2-3.5.1/ip/ip.c iproute2-3.5.1-tcp_metrics/ip/ip.c
--- iproute2-3.5.1/ip/ip.c	2012-08-13 18:13:58.000000000 +0300
+++ iproute2-3.5.1-tcp_metrics/ip/ip.c	2012-08-23 10:21:20.917653464 +0300
@@ -45,7 +45,7 @@ static void usage(void)
 "       ip [ -force ] -batch filename\n"
 "where  OBJECT := { link | addr | addrlabel | route | rule | neigh | ntable |\n"
 "                   tunnel | tuntap | maddr | mroute | mrule | monitor | xfrm |\n"
-"                   netns | l2tp }\n"
+"                   netns | l2tp | tcp_metrics }\n"
 "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n"
 "                    -f[amily] { inet | inet6 | ipx | dnet | link } |\n"
 "                    -l[oops] { maximum-addr-flush-attempts } |\n"
@@ -78,6 +78,8 @@ static const struct cmd {
 	{ "tunl",	do_iptunnel },
 	{ "tuntap",	do_iptuntap },
 	{ "tap",	do_iptuntap },
+	{ "tcpmetrics",	do_tcp_metrics },
+	{ "tcp_metrics",do_tcp_metrics },
 	{ "monitor",	do_ipmonitor },
 	{ "xfrm",	do_xfrm },
 	{ "mroute",	do_multiroute },
diff -urpN iproute2-3.5.1/ip/ip_common.h iproute2-3.5.1-tcp_metrics/ip/ip_common.h
--- iproute2-3.5.1/ip/ip_common.h	2012-08-13 18:13:58.000000000 +0300
+++ iproute2-3.5.1-tcp_metrics/ip/ip_common.h	2012-08-23 10:19:11.005647457 +0300
@@ -42,6 +42,7 @@ extern int do_multirule(int argc, char *
 extern int do_netns(int argc, char **argv);
 extern int do_xfrm(int argc, char **argv);
 extern int do_ipl2tp(int argc, char **argv);
+extern int do_tcp_metrics(int argc, char **argv);
 
 static inline int rtm_get_table(struct rtmsg *r, struct rtattr **tb)
 {
diff -urpN iproute2-3.5.1/ip/tcp_metrics.c iproute2-3.5.1-tcp_metrics/ip/tcp_metrics.c
--- iproute2-3.5.1/ip/tcp_metrics.c	1970-01-01 02:00:00.000000000 +0200
+++ iproute2-3.5.1-tcp_metrics/ip/tcp_metrics.c	2012-08-23 16:57:51.422753475 +0300
@@ -0,0 +1,498 @@
+/*
+ * tcp_metrics.c	"ip tcp_metrics/tcpmetrics"
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		version 2 as published by the Free Software Foundation;
+ *
+ * Authors:	Julian Anastasov <ja@ssi.bg>, August 2012
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <arpa/inet.h>
+#include <sys/ioctl.h>
+#include <linux/if.h>
+
+#include <linux/genetlink.h>
+#include <linux/tcp_metrics.h>
+
+#include "utils.h"
+#include "ip_common.h"
+
+static void usage(void)
+{
+	fprintf(stderr, "Usage: ip tcp_metrics/tcpmetrics { COMMAND | help }\n");
+	fprintf(stderr, "ip tcp_metrics { show | flush } SELECTOR\n");
+	fprintf(stderr, "ip tcp_metrics delete [ address ] ADDRESS\n");
+	fprintf(stderr, "SELECTOR := [ [ address ] PREFIX ]\n");
+	exit(-1);
+}
+
+/* netlink socket */
+static struct rtnl_handle grth = { .fd = -1 };
+static int genl_family = -1;
+
+
+#define GENL_DEFINE_REQUEST(req, hdrsize, bufsiz)			\
+struct {								\
+	struct nlmsghdr		n;					\
+	struct genlmsghdr	g;					\
+	char			buf[NLMSG_ALIGN(hdrsize) + bufsiz];	\
+} req
+
+#define GENL_INIT_REQUEST(req, family, hdrsize, ver, cmd_, flags)	\
+	do {								\
+		memset(&req, 0, sizeof(req));				\
+		req.n.nlmsg_type = family;				\
+		req.n.nlmsg_flags = flags;				\
+		req.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN + hdrsize);	\
+		req.g.cmd = cmd_;					\
+		req.g.version = ver;					\
+	} while (0)
+
+#define INIT_GENL_ACTION(req, cmd, ack)					\
+	GENL_INIT_REQUEST(req, genl_family, 0,				\
+			  TCP_METRICS_GENL_VERSION, cmd,		\
+			  NLM_F_REQUEST | ((ack) ? NLM_F_ACK : 0))
+
+#define INIT_GENL_DUMP(req)						\
+	GENL_INIT_REQUEST(req, genl_family, 0,				\
+			  TCP_METRICS_GENL_VERSION,			\
+			  TCP_METRICS_CMD_GET,				\
+			  NLM_F_DUMP | NLM_F_REQUEST)
+
+#define CMD_LIST	0x0001	/* list, lst, show		*/
+#define CMD_DEL		0x0002	/* delete, remove		*/
+#define CMD_FLUSH	0x0004	/* flush			*/
+
+static struct {
+	char	*name;
+	int	code;
+} cmds[] = {
+	{	"list",		CMD_LIST	},
+	{	"lst",		CMD_LIST	},
+	{	"show",		CMD_LIST	},
+	{	"delete",	CMD_DEL		},
+	{	"remove",	CMD_DEL		},
+	{	"flush",	CMD_FLUSH	},
+};
+
+static char *metric_name[TCP_METRIC_MAX + 1] = {
+	[TCP_METRIC_RTT]		= "rtt",
+	[TCP_METRIC_RTTVAR]		= "rttvar",
+	[TCP_METRIC_SSTHRESH]		= "ssthresh",
+	[TCP_METRIC_CWND]		= "cwnd",
+	[TCP_METRIC_REORDERING]		= "reordering",
+};
+
+static struct
+{
+	int flushed;
+	char *flushb;
+	int flushp;
+	int flushe;
+	int cmd;
+	inet_prefix addr;
+} f;
+
+static int flush_update(void)
+{
+	if (rtnl_send_check(&grth, f.flushb, f.flushp) < 0) {
+		perror("Failed to send flush request\n");
+		return -1;
+	}
+	f.flushp = 0;
+	return 0;
+}
+
+static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
+		       void *arg)
+{
+	FILE *fp = (FILE *) arg;
+	struct genlmsghdr *ghdr;
+	struct rtattr *attrs[TCP_METRICS_ATTR_MAX + 1], *a;
+	int len = n->nlmsg_len;
+	char abuf[256];
+	inet_prefix addr;
+	int family, i;
+
+	if (n->nlmsg_type != genl_family)
+		return -1;
+
+	len -= NLMSG_LENGTH(GENL_HDRLEN);
+	if (len < 0)
+		return -1;
+
+	ghdr = NLMSG_DATA(n);
+	if (ghdr->cmd != TCP_METRICS_CMD_GET)
+		return 0;
+
+	parse_rtattr(attrs, TCP_METRICS_ATTR_MAX, (void *) ghdr + GENL_HDRLEN,
+		     len);
+
+	a = attrs[TCP_METRICS_ATTR_ADDR_IPV4];
+	if (a) {
+		if (f.addr.family && f.addr.family != AF_INET)
+			return 0;
+		memcpy(&addr.data, RTA_DATA(a), 4);
+		family = AF_INET;
+	} else {
+		a = attrs[TCP_METRICS_ATTR_ADDR_IPV6];
+		if (a) {
+			if (f.addr.family && f.addr.family != AF_INET6)
+				return 0;
+			memcpy(&addr.data, RTA_DATA(a), 16);
+			family = AF_INET6;
+		} else
+			return 0;
+	}
+
+	if (f.addr.family && f.addr.bitlen >= 0 &&
+	    inet_addr_match(&addr, &f.addr, f.addr.bitlen))
+		return 0;
+
+	if (f.flushb) {
+		struct nlmsghdr *fn;
+		GENL_DEFINE_REQUEST(req2, 0, 128);
+
+		INIT_GENL_ACTION(req2, TCP_METRICS_CMD_DEL, 0);
+
+		if (NLMSG_ALIGN(f.flushp) + req2.n.nlmsg_len > f.flushe) {
+			if (flush_update())
+				return -1;
+		}
+		fn = (struct nlmsghdr *) (f.flushb + NLMSG_ALIGN(f.flushp));
+		memcpy(fn, &req2.n, req2.n.nlmsg_len);
+		fn->nlmsg_seq = ++grth.seq;
+		f.flushp = (((char *) fn) + req2.n.nlmsg_len) - f.flushb;
+		f.flushed++;
+		if (show_stats < 2)
+			return 0;
+	}
+
+	if (f.cmd & (CMD_DEL | CMD_FLUSH))
+		fprintf(fp, "Deleted ");
+
+	fprintf(fp, "%s",
+		format_host(family, RTA_PAYLOAD(a), &addr.data,
+			    abuf, sizeof(abuf)));
+
+	a = attrs[TCP_METRICS_ATTR_AGE];
+	if (a) {
+		__u64 val = rta_getattr_u64(a);
+
+		fprintf(fp, " age %llu.%03llusec",
+			val / 1000, val % 1000);
+	}
+
+	a = attrs[TCP_METRICS_ATTR_TW_TS_STAMP];
+	if (a) {
+		__s32 val = (__s32) rta_getattr_u32(a);
+		__u32 tsval;
+
+		a = attrs[TCP_METRICS_ATTR_TW_TSVAL];
+		tsval = a ? rta_getattr_u32(a) : 0;
+		fprintf(fp, " tw_ts %u/%dsec ago", tsval, val);
+	}
+
+	a = attrs[TCP_METRICS_ATTR_VALS];
+	if (a) {
+		struct rtattr *m[TCP_METRIC_MAX + 1 + 1];
+
+		parse_rtattr_nested(m, TCP_METRIC_MAX + 1, a);
+
+		for (i = 0; i < TCP_METRIC_MAX + 1; i++) {
+			__u32 val;
+
+			a = m[i + 1];
+			if (!a)
+				continue;
+			if (metric_name[i])
+				fprintf(fp, " %s ", metric_name[i]);
+			else
+				fprintf(fp, " metric_%d ", i);
+			val = rta_getattr_u32(a);
+			switch (i) {
+			case TCP_METRIC_RTT:
+			case TCP_METRIC_RTTVAR:
+				fprintf(fp, "%ums", val);
+				break;
+			case TCP_METRIC_SSTHRESH:
+			case TCP_METRIC_CWND:
+			case TCP_METRIC_REORDERING:
+			default:
+				fprintf(fp, "%u", val);
+				break;
+			}
+		}
+	}
+
+	a = attrs[TCP_METRICS_ATTR_FOPEN_MSS];
+	if (a)
+		fprintf(fp, " fo_mss %u", rta_getattr_u16(a));
+
+	a = attrs[TCP_METRICS_ATTR_FOPEN_SYN_DROPS];
+	if (a) {
+		__u16 syn_loss = rta_getattr_u16(a);
+		__u64 ts;
+
+		a = attrs[TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS];
+		ts = a ? rta_getattr_u64(a) : 0;
+
+		fprintf(fp, " fo_syn_drops %u/%llu.%03llusec ago",
+			syn_loss, ts / 1000, ts % 1000);
+	}
+
+	a = attrs[TCP_METRICS_ATTR_FOPEN_COOKIE];
+	if (a) {
+		char cookie[32 + 1];
+		unsigned char *ptr = RTA_DATA(a);
+		int i, max = RTA_PAYLOAD(a);
+
+		if (max > 16)
+			max = 16;
+		cookie[0] = 0;
+		for (i = 0; i < max; i++)
+			sprintf(cookie + i + i, "%02x", ptr[i]);
+		fprintf(fp, " fo_cookie %s", cookie);
+	}
+
+	fprintf(fp, "\n");
+
+	fflush(fp);
+	return 0;
+}
+
+static int genl_parse_getfamily(struct nlmsghdr *nlh)
+{
+	struct rtattr *tb[CTRL_ATTR_MAX + 1];
+	struct genlmsghdr *ghdr = NLMSG_DATA(nlh);
+	int len = nlh->nlmsg_len;
+	struct rtattr *attrs;
+
+	if (nlh->nlmsg_type != GENL_ID_CTRL) {
+		fprintf(stderr, "Not a controller message, nlmsg_len=%d "
+			"nlmsg_type=0x%x\n", nlh->nlmsg_len, nlh->nlmsg_type);
+		return -1;
+	}
+
+	if (ghdr->cmd != CTRL_CMD_NEWFAMILY) {
+		fprintf(stderr, "Unknown controller command %d\n", ghdr->cmd);
+		return -1;
+	}
+
+	len -= NLMSG_LENGTH(GENL_HDRLEN);
+
+	if (len < 0) {
+		fprintf(stderr, "wrong controller message len %d\n", len);
+		return -1;
+	}
+
+	attrs = (struct rtattr *) ((char *) ghdr + GENL_HDRLEN);
+	parse_rtattr(tb, CTRL_ATTR_MAX, attrs, len);
+
+	if (tb[CTRL_ATTR_FAMILY_ID] == NULL) {
+		fprintf(stderr, "Missing family id TLV\n");
+		return -1;
+	}
+
+	return rta_getattr_u16(tb[CTRL_ATTR_FAMILY_ID]);
+}
+
+static int genl_ctrl_resolve_family(const char *family)
+{
+	GENL_DEFINE_REQUEST(req, 0, 1024);
+
+	GENL_INIT_REQUEST(req, GENL_ID_CTRL, 0, 0, CTRL_CMD_GETFAMILY,
+			  NLM_F_REQUEST);
+
+	addattr_l(&req.n, 1024, CTRL_ATTR_FAMILY_NAME,
+		  family, strlen(family) + 1);
+
+	if (rtnl_talk(&grth, &req.n, 0, 0, &req.n) < 0) {
+		fprintf(stderr, "Error talking to the kernel\n");
+		return -2;
+	}
+
+	return genl_parse_getfamily(&req.n);
+}
+
+static int tcpm_do_cmd(int cmd, int argc, char **argv)
+{
+	GENL_DEFINE_REQUEST(req, 0, 1024);
+	int atype = -1;
+	int code, ack;
+
+	memset(&f, 0, sizeof(f));
+	f.addr.bitlen = -1;
+	f.addr.family = preferred_family;
+
+	switch (preferred_family) {
+	case AF_UNSPEC:
+	case AF_INET:
+	case AF_INET6:
+		break;
+	default:
+		fprintf(stderr, "Unsupported family:%d\n", preferred_family);
+		return -1;
+	}
+
+	for (; argc > 0; argc--, argv++) {
+		char *who = "address";
+
+		if (strcmp(*argv, "addr") == 0 ||
+		    strcmp(*argv, "address") == 0) {
+			who = *argv;
+			NEXT_ARG();
+		}
+		if (matches(*argv, "help") == 0)
+			usage();
+		if (f.addr.bitlen >= 0)
+			duparg2(who, *argv);
+
+		get_prefix(&f.addr, *argv, preferred_family);
+		if (f.addr.bytelen && f.addr.bytelen * 8 == f.addr.bitlen) {
+			if (f.addr.family == AF_INET)
+				atype = TCP_METRICS_ATTR_ADDR_IPV4;
+			else if (f.addr.family == AF_INET6)
+				atype = TCP_METRICS_ATTR_ADDR_IPV6;
+		}
+		if ((CMD_DEL & cmd) && atype < 0) {
+			fprintf(stderr, "Error: a specific IP address is expected rather than \"%s\"\n",
+				*argv);
+			return -1;
+		}
+
+		argc--; argv++;
+	}
+
+	if (cmd == CMD_DEL && atype < 0)
+		missarg("address");
+
+	/* flush for exact address ? Single del */
+	if (cmd == CMD_FLUSH && atype >= 0)
+		cmd = CMD_DEL;
+	/* flush for all addresses ? Single del without address */
+	if (cmd == CMD_FLUSH && f.addr.bitlen < 0 &&
+	    preferred_family == AF_UNSPEC) {
+		cmd = CMD_DEL;
+		code = TCP_METRICS_CMD_DEL;
+		ack = 1;
+	} else if (cmd == CMD_DEL) {
+		code = TCP_METRICS_CMD_DEL;
+		ack = 1;
+	} else {	/* CMD_FLUSH, CMD_LIST */
+		code = TCP_METRICS_CMD_GET;
+		ack = 0;
+	}
+
+	if (genl_family < 0) {
+		if (rtnl_open_byproto(&grth, 0, NETLINK_GENERIC) < 0) {
+			fprintf(stderr, "Cannot open generic netlink socket\n");
+			exit(1);
+		}
+		genl_family = genl_ctrl_resolve_family(TCP_METRICS_GENL_NAME);
+		if (genl_family < 0)
+			exit(1);
+	}
+
+	if (!(cmd & CMD_FLUSH) && (atype >= 0 || (cmd & CMD_DEL))) {
+		INIT_GENL_ACTION(req, code, ack);
+		if (atype >= 0)
+			addattr_l(&req.n, sizeof(req), atype, &f.addr.data,
+				  f.addr.bytelen);
+	} else {
+		INIT_GENL_DUMP(req);
+	}
+
+	f.cmd = cmd;
+	if (cmd & CMD_FLUSH) {
+		int round = 0;
+		char flushb[4096-512];
+
+		f.flushb = flushb;
+		f.flushp = 0;
+		f.flushe = sizeof(flushb);
+
+		for (;;) {
+			req.n.nlmsg_seq = grth.dump = ++grth.seq;
+			if (rtnl_send(&grth, &req, req.n.nlmsg_len) < 0) {
+				perror("Failed to send flush request");
+				exit(1);
+			}
+			f.flushed = 0;
+			if (rtnl_dump_filter(&grth, process_msg, stdout) < 0) {
+				fprintf(stderr, "Flush terminated\n");
+				exit(1);
+			}
+			if (f.flushed == 0) {
+				if (round == 0) {
+					fprintf(stderr, "Nothing to flush.\n");
+				} else if (show_stats)
+					printf("*** Flush is complete after %d round%s ***\n",
+					       round, round > 1 ? "s" : "");
+				fflush(stdout);
+				return 0;
+			}
+			round++;
+			if (flush_update() < 0)
+				exit(1);
+			if (show_stats) {
+				printf("\n*** Round %d, deleting %d entries ***\n",
+				       round, f.flushed);
+				fflush(stdout);
+			}
+		}
+		return 0;
+	}
+
+	req.n.nlmsg_seq = ++grth.seq;
+	if (ack) {
+		if (rtnl_talk(&grth, &req.n, 0, 0, NULL) < 0)
+			return -2;
+	} else if (atype >= 0) {
+		if (rtnl_talk(&grth, &req.n, 0, 0, &req.n) < 0)
+			return -2;
+		if (process_msg(NULL, &req.n, stdout) < 0) {
+			fprintf(stderr, "Dump terminated\n");
+			exit(1);
+		}
+	} else {
+		grth.dump = grth.seq;
+		if (rtnl_send(&grth, &req, req.n.nlmsg_len) < 0) {
+			perror("Failed to send dump request");
+			exit(1);
+		}
+
+		if (rtnl_dump_filter(&grth, process_msg, stdout) < 0) {
+			fprintf(stderr, "Dump terminated\n");
+			exit(1);
+		}
+	}
+	return 0;
+}
+
+int do_tcp_metrics(int argc, char **argv)
+{
+	int i;
+
+	if (argc < 1)
+		return tcpm_do_cmd(CMD_LIST, 0, NULL);
+	for (i = 0; i < ARRAY_SIZE(cmds); i++) {
+		if (matches(argv[0], cmds[i].name) == 0)
+			return tcpm_do_cmd(cmds[i].code, argc-1, argv+1);
+	}
+	if (matches(argv[0], "help") == 0)
+		usage();
+
+	fprintf(stderr, "Command \"%s\" is unknown, "
+			"try \"ip tcp_metrics help\".\n", *argv);
+	exit(-1);
+}
+
diff -urpN iproute2-3.5.1/man/man8/Makefile iproute2-3.5.1-tcp_metrics/man/man8/Makefile
--- iproute2-3.5.1/man/man8/Makefile	2012-08-13 18:13:58.000000000 +0300
+++ iproute2-3.5.1-tcp_metrics/man/man8/Makefile	2012-08-23 14:45:12.506385476 +0300
@@ -7,7 +7,8 @@ MAN8PAGES = $(TARGETS) ip.8 arpd.8 lnsta
 	bridge.8 rtstat.8 ctstat.8 nstat.8 routef.8 \
 	ip-tunnel.8 ip-rule.8 ip-ntable.8 \
 	ip-monitor.8 tc-stab.8 tc-hfsc.8 ip-xfrm.8 ip-netns.8 \
-	ip-neighbour.8 ip-mroute.8 ip-maddress.8 ip-addrlabel.8 
+	ip-neighbour.8 ip-mroute.8 ip-maddress.8 ip-addrlabel.8 \
+	ip-tcp_metrics.8
 
 
 all: $(TARGETS)
diff -urpN iproute2-3.5.1/man/man8/ip-tcp_metrics.8 iproute2-3.5.1-tcp_metrics/man/man8/ip-tcp_metrics.8
--- iproute2-3.5.1/man/man8/ip-tcp_metrics.8	1970-01-01 02:00:00.000000000 +0200
+++ iproute2-3.5.1-tcp_metrics/man/man8/ip-tcp_metrics.8	2012-08-23 14:48:09.262393650 +0300
@@ -0,0 +1,96 @@
+.TH "IP\-TCP_METRICS" 8 "23 Aug 2012" "iproute2" "Linux"
+.SH "NAME"
+ip-tcp_metrics \- management for TCP Metrics
+.SH "SYNOPSIS"
+.sp
+.ad l
+.in +8
+.ti -8
+.B ip
+.RI "[ " OPTIONS " ]"
+.B tcp_metrics
+.RI "{ " COMMAND " | "
+.BR help " }"
+.sp
+
+.ti -8
+.BR "ip tcp_metrics" " { " show " | " flush " }
+.IR SELECTOR
+
+.ti -8
+.BR "ip tcp_metrics delete " [ " address " ]
+.IR ADDRESS
+
+.ti -8
+.IR SELECTOR " := " 
+.RB "[ [ " address " ] "
+.IR PREFIX " ]"
+
+.SH "DESCRIPTION"
+.B ip tcp_metrics
+is used to manipulate entries in the kernel that keep TCP information
+for IPv4 or IPv6 destinations. The entries are created when
+TCP sockets want to share information for destinations and are
+stored in cache keyed by the destination address. For performance
+reasons the cache can not grow above configured limit and the
+older entries are replaced with fresh information, sometimes
+reclaimed and used for new destinations. The kernel never removes
+entries, so they can be flushed only with this tool.
+
+.SS ip tcp_metrics show - show cached entries
+
+.TP
+.BI address " PREFIX " (default)
+IPv4/IPv6 prefix or address. If no prefix is provided all entries are shown.
+
+.SS ip tcp_metrics delete - delete single entry
+
+.TP
+.BI address " ADDRESS " (default)
+IPv4/IPv6 address. The address is a required argument.
+
+.SS ip tcp_metrics flush - flush entries
+This command flushes the entries selected by some criteria.
+
+.PP
+This command has the same arguments as
+.B show.
+
+.SH "EXAMPLES"
+.PP
+ip tcp_metrics show address 192.168.0.0/24
+.RS 4
+Shows the entries for destinations from subnet
+.RE
+.PP
+ip tcp_metrics show 192.168.0.0/24
+.RS 4
+The same but address keyword is optional
+.RE
+.PP
+ip tcp_metrics delete 192.168.0.1
+.RS 4
+Removes the entry for 192.168.0.1 from cache.
+.RE
+.PP
+ip tcp_metrics flush 192.168.0.0/24
+.RS 4
+Removes entries for destinations from subnet
+.RE
+.PP
+ip tcp_metrics flush all
+.RS 4
+Removes all entries from cache
+.RE
+.PP
+ip -6 tcp_metrics flush all
+.RS 4
+Removes all IPv6 entries from cache keeping the IPv4 entries.
+.RE
+
+.SH SEE ALSO
+.br
+.BR ip (8)
+
+.SH AUTHOR
+Original Manpage by Julian Anastasov <ja@ssi.bg>
diff -urpN iproute2-3.5.1/man/man8/ip.8 iproute2-3.5.1-tcp_metrics/man/man8/ip.8
--- iproute2-3.5.1/man/man8/ip.8	2012-08-13 18:13:58.000000000 +0300
+++ iproute2-3.5.1-tcp_metrics/man/man8/ip.8	2012-08-23 10:28:12.541672495 +0300
@@ -15,7 +15,7 @@ ip \- show / manipulate routing, devices
 .IR OBJECT " := { "
 .BR link " | " addr " | " addrlabel " | " route " | " rule " | " neigh " | "\
  ntable " | " tunnel " | " tuntap " | " maddr " | "  mroute " | " mrule " | "\
- monitor " | " xfrm " | " netns " | "  l2tp " }"
+ monitor " | " xfrm " | " netns " | "  l2tp " | "  tcp_metrics " }"
 .sp
 
 .ti -8
@@ -156,6 +156,10 @@ host addresses.
 - rule in routing policy database.
 
 .TP
+.B tcp_metrics/tcpmetrics
+- manage TCP Metrics
+
+.TP
 .B tunnel
 - tunnel over IP.
 
@@ -215,6 +219,7 @@ was written by Alexey N. Kuznetsov and a
 .BR ip-ntable (8),
 .BR ip-route (8),
 .BR ip-rule (8),
+.BR ip-tcp_metrics (8),
 .BR ip-tunnel (8),
 .BR ip-xfrm (8)
 .br

^ permalink raw reply

* [PATCH RFC 1/2] tcp: add generic netlink support for tcp_metrics
From: Julian Anastasov @ 2012-08-23 15:01 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1345734105-28328-1-git-send-email-ja@ssi.bg>

	Add support for genl "tcp_metrics". No locking
is changed, only that now we can unlink and delete
entries after grace period. We implement get/del for
single entry and dump to support show/flush filtering
in user space.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 include/linux/Kbuild        |    1 +
 include/linux/tcp_metrics.h |   54 +++++++
 net/ipv4/tcp_metrics.c      |  353 +++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 395 insertions(+), 13 deletions(-)
 create mode 100644 include/linux/tcp_metrics.h

diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index 1f2c1c7..90da0af 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -363,6 +363,7 @@ header-y += sysctl.h
 header-y += sysinfo.h
 header-y += taskstats.h
 header-y += tcp.h
+header-y += tcp_metrics.h
 header-y += telephony.h
 header-y += termios.h
 header-y += time.h
diff --git a/include/linux/tcp_metrics.h b/include/linux/tcp_metrics.h
new file mode 100644
index 0000000..cb5157b
--- /dev/null
+++ b/include/linux/tcp_metrics.h
@@ -0,0 +1,54 @@
+/* tcp_metrics.h - TCP Metrics Interface */
+
+#ifndef _LINUX_TCP_METRICS_H
+#define _LINUX_TCP_METRICS_H
+
+#include <linux/types.h>
+
+/* NETLINK_GENERIC related info
+ */
+#define TCP_METRICS_GENL_NAME		"tcp_metrics"
+#define TCP_METRICS_GENL_VERSION	0x1
+
+enum tcp_metric_index {
+	TCP_METRIC_RTT,
+	TCP_METRIC_RTTVAR,
+	TCP_METRIC_SSTHRESH,
+	TCP_METRIC_CWND,
+	TCP_METRIC_REORDERING,
+
+	/* Always last.  */
+	__TCP_METRIC_MAX,
+};
+
+#define TCP_METRIC_MAX	(__TCP_METRIC_MAX - 1)
+
+enum {
+	TCP_METRICS_ATTR_UNSPEC,
+	TCP_METRICS_ATTR_ADDR_IPV4,		/* u32 */
+	TCP_METRICS_ATTR_ADDR_IPV6,		/* binary */
+	TCP_METRICS_ATTR_AGE,			/* msecs */
+	TCP_METRICS_ATTR_TW_TSVAL,		/* u32, raw, rcv tsval */
+	TCP_METRICS_ATTR_TW_TS_STAMP,		/* s32, sec age */
+	TCP_METRICS_ATTR_VALS,			/* nested +1, u32 */
+	TCP_METRICS_ATTR_FOPEN_MSS,		/* u16 */
+	TCP_METRICS_ATTR_FOPEN_SYN_DROPS,	/* u16, count of drops */
+	TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS,	/* msecs age */
+	TCP_METRICS_ATTR_FOPEN_COOKIE,		/* binary */
+
+	__TCP_METRICS_ATTR_MAX,
+};
+
+#define TCP_METRICS_ATTR_MAX	(__TCP_METRICS_ATTR_MAX - 1)
+
+enum {
+	TCP_METRICS_CMD_UNSPEC,
+	TCP_METRICS_CMD_GET,
+	TCP_METRICS_CMD_DEL,
+
+	__TCP_METRICS_CMD_MAX,
+};
+
+#define TCP_METRICS_CMD_MAX	(__TCP_METRICS_CMD_MAX - 1)
+
+#endif /* _LINUX_TCP_METRICS_H */
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 0abe67b..6abe64a 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -8,6 +8,7 @@
 #include <linux/init.h>
 #include <linux/tcp.h>
 #include <linux/hash.h>
+#include <linux/tcp_metrics.h>
 
 #include <net/inet_connection_sock.h>
 #include <net/net_namespace.h>
@@ -17,20 +18,10 @@
 #include <net/ipv6.h>
 #include <net/dst.h>
 #include <net/tcp.h>
+#include <net/genetlink.h>
 
 int sysctl_tcp_nometrics_save __read_mostly;
 
-enum tcp_metric_index {
-	TCP_METRIC_RTT,
-	TCP_METRIC_RTTVAR,
-	TCP_METRIC_SSTHRESH,
-	TCP_METRIC_CWND,
-	TCP_METRIC_REORDERING,
-
-	/* Always last.  */
-	TCP_METRIC_MAX,
-};
-
 struct tcp_fastopen_metrics {
 	u16	mss;
 	u16	syn_loss:10;		/* Recurring Fast Open SYN losses */
@@ -45,8 +36,10 @@ struct tcp_metrics_block {
 	u32				tcpm_ts;
 	u32				tcpm_ts_stamp;
 	u32				tcpm_lock;
-	u32				tcpm_vals[TCP_METRIC_MAX];
+	u32				tcpm_vals[TCP_METRIC_MAX + 1];
 	struct tcp_fastopen_metrics	tcpm_fastopen;
+
+	struct rcu_head			rcu_head;
 };
 
 static bool tcp_metric_locked(struct tcp_metrics_block *tm,
@@ -690,6 +683,324 @@ void tcp_fastopen_cache_set(struct sock *sk, u16 mss,
 	rcu_read_unlock();
 }
 
+static struct genl_family tcp_metrics_nl_family = {
+	.id		= GENL_ID_GENERATE,
+	.hdrsize	= 0,
+	.name		= TCP_METRICS_GENL_NAME,
+	.version	= TCP_METRICS_GENL_VERSION,
+	.maxattr	= TCP_METRICS_ATTR_MAX,
+	.netnsok	= true,
+};
+
+static struct nla_policy tcp_metrics_nl_policy[TCP_METRICS_ATTR_MAX + 1] = {
+	[TCP_METRICS_ATTR_ADDR_IPV4]	= { .type = NLA_U32, },
+	[TCP_METRICS_ATTR_ADDR_IPV6]	= { .type = NLA_BINARY,
+					    .len = sizeof(struct in6_addr), },
+	/* Following attributes are not received for GET/DEL,
+	 * we keep them for reference
+	 */
+#if 0
+	[TCP_METRICS_ATTR_AGE]		= { .type = NLA_MSECS, },
+	[TCP_METRICS_ATTR_TW_TSVAL]	= { .type = NLA_U32, },
+	[TCP_METRICS_ATTR_TW_TS_STAMP]	= { .type = NLA_S32, },
+	[TCP_METRICS_ATTR_VALS]		= { .type = NLA_NESTED, },
+	[TCP_METRICS_ATTR_FOPEN_MSS]	= { .type = NLA_U16, },
+	[TCP_METRICS_ATTR_FOPEN_SYN_DROPS]	= { .type = NLA_U16, },
+	[TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS]	= { .type = NLA_MSECS, },
+	[TCP_METRICS_ATTR_FOPEN_COOKIE]	= { .type = NLA_BINARY,
+					    .len = TCP_FASTOPEN_COOKIE_MAX, },
+#endif
+};
+
+/* Add attributes, caller cancels its header on failure */
+static int tcp_metrics_fill_info(struct sk_buff *msg,
+				 struct tcp_metrics_block *tm)
+{
+	struct nlattr *nest;
+	int i;
+
+	switch (tm->tcpm_addr.family) {
+	case AF_INET:
+		if (nla_put_be32(msg, TCP_METRICS_ATTR_ADDR_IPV4,
+				tm->tcpm_addr.addr.a4) < 0)
+			goto nla_put_failure;
+		break;
+	case AF_INET6:
+		if (nla_put(msg, TCP_METRICS_ATTR_ADDR_IPV6, 16,
+			    tm->tcpm_addr.addr.a6) < 0)
+			goto nla_put_failure;
+		break;
+	default:
+		return -EAFNOSUPPORT;
+	}
+
+	if (nla_put_msecs(msg, TCP_METRICS_ATTR_AGE,
+			  jiffies - tm->tcpm_stamp) < 0)
+		goto nla_put_failure;
+	if (tm->tcpm_ts_stamp) {
+		if (nla_put_s32(msg, TCP_METRICS_ATTR_TW_TS_STAMP,
+				(s32) (get_seconds() - tm->tcpm_ts_stamp)) < 0)
+			goto nla_put_failure;
+		if (nla_put_u32(msg, TCP_METRICS_ATTR_TW_TSVAL,
+				tm->tcpm_ts) < 0)
+			goto nla_put_failure;
+	}
+
+	{
+		int n = 0;
+
+		nest = nla_nest_start(msg, TCP_METRICS_ATTR_VALS);
+		if (!nest)
+			goto nla_put_failure;
+		for (i = 0; i < TCP_METRIC_MAX + 1; i++) {
+			if (!tm->tcpm_vals[i])
+				continue;
+			if (nla_put_u32(msg, i + 1, tm->tcpm_vals[i]) < 0)
+				goto nla_put_failure;
+			n++;
+		}
+		if (n)
+			nla_nest_end(msg, nest);
+		else
+			nla_nest_cancel(msg, nest);
+	}
+
+	{
+		struct tcp_fastopen_metrics tfom_copy[1], *tfom;
+		unsigned int seq;
+
+		do {
+			seq = read_seqbegin(&fastopen_seqlock);
+			tfom_copy[0] = tm->tcpm_fastopen;
+		} while (read_seqretry(&fastopen_seqlock, seq));
+
+		tfom = tfom_copy;
+		if (tfom->mss &&
+		    nla_put_u16(msg, TCP_METRICS_ATTR_FOPEN_MSS,
+				tfom->mss) < 0)
+			goto nla_put_failure;
+		if (tfom->syn_loss &&
+		    (nla_put_u16(msg, TCP_METRICS_ATTR_FOPEN_SYN_DROPS,
+				tfom->syn_loss) < 0 ||
+		     nla_put_msecs(msg, TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS,
+				jiffies - tfom->last_syn_loss) < 0))
+			goto nla_put_failure;
+		if (tfom->cookie.len > 0 &&
+		    nla_put(msg, TCP_METRICS_ATTR_FOPEN_COOKIE,
+			    tfom->cookie.len, tfom->cookie.val) < 0)
+			goto nla_put_failure;
+	}
+
+	return 0;
+
+nla_put_failure:
+	return -EMSGSIZE;
+}
+
+static int tcp_metrics_dump_info(struct sk_buff *skb,
+				 struct netlink_callback *cb,
+				 struct tcp_metrics_block *tm)
+{
+	void *hdr;
+
+	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq,
+			  &tcp_metrics_nl_family, NLM_F_MULTI,
+			  TCP_METRICS_CMD_GET);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (tcp_metrics_fill_info(skb, tm) < 0)
+		goto nla_put_failure;
+
+	return genlmsg_end(skb, hdr);
+
+nla_put_failure:
+	genlmsg_cancel(skb, hdr);
+	return -EMSGSIZE;
+}
+
+static int tcp_metrics_nl_dump(struct sk_buff *skb,
+			       struct netlink_callback *cb)
+{
+	struct net *net = sock_net(skb->sk);
+	unsigned int max_rows = 1U << net->ipv4.tcp_metrics_hash_log;
+	unsigned int row, s_row = cb->args[0];
+	int s_col = cb->args[1], col = s_col;
+
+	for (row = s_row; row < max_rows; row++, s_col = 0) {
+		struct tcp_metrics_block *tm;
+		struct tcpm_hash_bucket *hb = net->ipv4.tcp_metrics_hash + row;
+
+		rcu_read_lock();
+		for (col = 0, tm = rcu_dereference(hb->chain); tm;
+		     tm = rcu_dereference(tm->tcpm_next), col++) {
+			if (col < s_col)
+				continue;
+			if (tcp_metrics_dump_info(skb, cb, tm) < 0) {
+				rcu_read_unlock();
+				goto done;
+			}
+		}
+		rcu_read_unlock();
+	}
+
+done:
+	cb->args[0] = row;
+	cb->args[1] = col;
+	return skb->len;
+}
+
+static int parse_nl_addr(struct genl_info *info, struct inetpeer_addr *addr,
+			 unsigned int *hash, int optional)
+{
+	struct nlattr *a;
+
+	a = info->attrs[TCP_METRICS_ATTR_ADDR_IPV4];
+	if (a) {
+		addr->family = AF_INET;
+		addr->addr.a4 = nla_get_be32(a);
+		*hash = (__force unsigned int) addr->addr.a4;
+		return 0;
+	}
+	a = info->attrs[TCP_METRICS_ATTR_ADDR_IPV6];
+	if (a) {
+		if (nla_len(a) != sizeof(sizeof(struct in6_addr)))
+			return -EINVAL;
+		addr->family = AF_INET6;
+		memcpy(addr->addr.a6, nla_data(a), sizeof(addr->addr.a6));
+		*hash = ipv6_addr_hash((struct in6_addr *) addr->addr.a6);
+		return 0;
+	}
+	return optional ? 1 : -EAFNOSUPPORT;
+}
+
+static int tcp_metrics_nl_cmd_get(struct sk_buff *skb, struct genl_info *info)
+{
+	struct tcp_metrics_block *tm;
+	struct inetpeer_addr addr;
+	unsigned int hash;
+	struct sk_buff *msg;
+	struct net *net = genl_info_net(info);
+	void *reply;
+	int ret;
+
+	ret = parse_nl_addr(info, &addr, &hash, 0);
+	if (ret < 0)
+		return ret;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	reply = genlmsg_put_reply(msg, info, &tcp_metrics_nl_family, 0,
+				  info->genlhdr->cmd);
+	if (!reply)
+		goto nla_put_failure;
+
+	hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
+	ret = -ESRCH;
+	rcu_read_lock();
+	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
+	     tm = rcu_dereference(tm->tcpm_next)) {
+		if (addr_same(&tm->tcpm_addr, &addr)) {
+			ret = tcp_metrics_fill_info(msg, tm);
+			break;
+		}
+	}
+	rcu_read_unlock();
+	if (ret < 0)
+		goto out_free;
+
+	genlmsg_end(msg, reply);
+	return genlmsg_reply(msg, info);
+
+nla_put_failure:
+	ret = -EMSGSIZE;
+
+out_free:
+	nlmsg_free(msg);
+	return ret;
+}
+
+#define deref_locked_genl(p)	\
+	rcu_dereference_protected(p, lockdep_genl_is_held() && \
+				     lockdep_is_held(&tcp_metrics_lock))
+
+#define deref_genl(p)	rcu_dereference_protected(p, lockdep_genl_is_held())
+
+static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
+{
+	struct tcpm_hash_bucket *hb;
+	struct tcp_metrics_block *tm;
+	struct tcp_metrics_block __rcu **pp;
+	struct inetpeer_addr addr;
+	unsigned int hash;
+	struct net *net = genl_info_net(info);
+	int ret;
+
+	ret = parse_nl_addr(info, &addr, &hash, 1);
+	if (ret < 0)
+		return ret;
+	/* Flush all ? */
+	if (ret > 0) {
+		unsigned int max_rows = 1U << net->ipv4.tcp_metrics_hash_log;
+		unsigned int sync_count = 0;
+		unsigned int row;
+
+		hb = net->ipv4.tcp_metrics_hash;
+		for (row = 0; row < max_rows; row++, hb++) {
+			spin_lock_bh(&tcp_metrics_lock);
+			tm = deref_locked_genl(hb->chain);
+			if (tm)
+				rcu_assign_pointer(hb->chain, NULL);
+			spin_unlock_bh(&tcp_metrics_lock);
+			while (tm) {
+				struct tcp_metrics_block *next;
+
+				next = deref_genl(tm->tcpm_next);
+				kfree_rcu(tm, rcu_head);
+				if (!((++sync_count) & 2047))
+					synchronize_rcu();
+				tm = next;
+			}
+		}
+		return 0;
+	}
+
+	hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
+	hb = net->ipv4.tcp_metrics_hash + hash;
+	pp = &hb->chain;
+	spin_lock_bh(&tcp_metrics_lock);
+	for (tm = deref_locked_genl(*pp); tm;
+	     pp = &tm->tcpm_next, tm = deref_locked_genl(*pp)) {
+		if (addr_same(&tm->tcpm_addr, &addr)) {
+			rcu_assign_pointer(*pp, tm->tcpm_next);
+			break;
+		}
+	}
+	spin_unlock_bh(&tcp_metrics_lock);
+	if (!tm)
+		return -ESRCH;
+	kfree_rcu(tm, rcu_head);
+	return 0;
+}
+
+static struct genl_ops tcp_metrics_nl_ops[] = {
+	{
+		.cmd = TCP_METRICS_CMD_GET,
+		.doit = tcp_metrics_nl_cmd_get,
+		.dumpit = tcp_metrics_nl_dump,
+		.policy = tcp_metrics_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+	},
+	{
+		.cmd = TCP_METRICS_CMD_DEL,
+		.doit = tcp_metrics_nl_cmd_del,
+		.policy = tcp_metrics_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+	},
+};
+
 static unsigned int tcpmhash_entries;
 static int __init set_tcpmhash_entries(char *str)
 {
@@ -753,5 +1064,21 @@ static __net_initdata struct pernet_operations tcp_net_metrics_ops = {
 
 void __init tcp_metrics_init(void)
 {
-	register_pernet_subsys(&tcp_net_metrics_ops);
+	int ret;
+
+	ret = register_pernet_subsys(&tcp_net_metrics_ops);
+	if (ret < 0)
+		goto cleanup;
+	ret = genl_register_family_with_ops(&tcp_metrics_nl_family,
+					    tcp_metrics_nl_ops,
+					    ARRAY_SIZE(tcp_metrics_nl_ops));
+	if (ret < 0)
+		goto cleanup_subsys;
+	return;
+
+cleanup_subsys:
+	unregister_pernet_subsys(&tcp_net_metrics_ops);
+
+cleanup:
+	return;
 }
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH RFC 0/2] Interface for TCP Metrics
From: Julian Anastasov @ 2012-08-23 15:01 UTC (permalink / raw)
  To: netdev

	This patchset contains 2 patches, one for kernel
and one for iproute2. We add DUMP/GET/DEL support for
genl "tcp_metrics" and minimal support for filtering
by address prefix and family in user space.

	I tested show/del/flush, filtering by family, prefix,
output for metrics such as rtt, rttvar, cwnd, ssthresh (after
modifying route). I didn't tested output for fast open.

	May be some corrections in output can be desired.

	The kernel patch has some parts to check:

- in tcp_metrics_nl_cmd_del I'm trying to flush all addresses
with single request, eg. when no family, address or other
selectors are provided. I use some arbitrary counter sync_count
to force memory to be freed in parts by using synchronize_rcu,
note that we are under genl_mutex, so may be this is bad idea
to delay other genl users? My idea was to avoid many commands
on "flush all", may be we should trigger flush by using some
other mechanism that is pernet? And I don't know how to test
it properly without large cache.

- This function now uses rcu_dereference_protected, someone
should check if these lockdep checks are correct. I enabled
lockdep in config and don't see any warnings.

^ permalink raw reply

* Re: Re[2]:  [PATCH 05/19] netfilter: nf_conntrack_ipv6: improve fragmentation handling
From: Jesper Dangaard Brouer @ 2012-08-23 14:49 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: Hans Schillstrom, Patrick McHardy, netfilter-devel, netdev,
	Hans Schillstrom
In-Reply-To: <alpine.LFD.2.00.1208221014000.1698@ja.ssi.bg>

On Wed, 2012-08-22 at 10:16 +0300, Julian Anastasov wrote:
> 	Hello,
> 
> On Wed, 22 Aug 2012, Julian Anastasov wrote:
> 
> > On Wed, 22 Aug 2012, Hans Schillstrom wrote:
> > 
> > > >Perhaps we could change/fix the MTU check in IPVS?
> > > >(This would also solve issues I've seen with TSO/GSO frames, hitting
> > > >this code path).
> > > >
> > > I ran into this as well, 
> > > try this for the mtu check.
> > > 
> > >        if ((!skb->local_df && skb->len > mtu && !skb_is_gso(skb)) ||
> > >            (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)) {
> > 
> > 	Better without local_df check, it is our job to
> > set it.

Will the IPVS usage of local_df, conflict with Patrick's usage?



> 	Ops, sorry. It seems now nf_ct_frag6_reasm (Patch 02/18)
> will set head->local_df = 1, so we should check local_df
> as Hans said.

With Hans'es version of the MTU check, IPVS seems to work again (with
Patricks patches applied).

(Also spotted the GSO problem I had with IPv6 + DR, and send an upstream
fix...)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer



^ permalink raw reply

* Re: [PATCH] netvm: check for page == NULL when propogating the skb->pfmemalloc flag
From: Konrad Rzeszutek Wilk @ 2012-08-23 14:17 UTC (permalink / raw)
  To: Ian Campbell
  Cc: David Miller, mgorman@suse.de, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	xen-devel@lists.xensource.com, konrad@darnok.org,
	akpm@linux-foundation.org
In-Reply-To: <1345631207.6821.140.camel@zakaz.uk.xensource.com>

On Wed, Aug 22, 2012 at 11:26:47AM +0100, Ian Campbell wrote:
> On Wed, 2012-08-08 at 23:50 +0100, David Miller wrote:
> > Just use something like a call to __pskb_pull_tail(skb, len) and all
> > that other crap around that area can simply be deleted.
> 
> I think you mean something like this, which works for me, although I've
> only lightly tested it.
> 

I've tested it heavily and works great.

Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
and I took a look at it too and:

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Ian.
> 
> 8<----------------------------------------
> 
> >From 9e47e3e87a33b45974448649a97859a479183041 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Wed, 22 Aug 2012 10:15:29 +0100
> Subject: [PATCH] xen-netfront: use __pskb_pull_tail to ensure linear area is big enough on RX
> 
> I'm slightly concerned by the "only in exceptional circumstances"
> comment on __pskb_pull_tail but the structure of an skb just created
> by netfront shouldn't hit any of the especially slow cases.
> 
> This approach still does slightly more work than the old way, since if
> we pull up the entire first frag we now have to shuffle everything
> down where before we just received into the right place in the first
> place.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: xen-devel@lists.xensource.com
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/net/xen-netfront.c |   39 ++++++++++-----------------------------
>  1 files changed, 10 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 3089990..650f79a 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -57,8 +57,7 @@
>  static const struct ethtool_ops xennet_ethtool_ops;
>  
>  struct netfront_cb {
> -	struct page *page;
> -	unsigned offset;
> +	int pull_to;
>  };
>  
>  #define NETFRONT_SKB_CB(skb)	((struct netfront_cb *)((skb)->cb))
> @@ -867,15 +866,9 @@ static int handle_incoming_queue(struct net_device *dev,
>  	struct sk_buff *skb;
>  
>  	while ((skb = __skb_dequeue(rxq)) != NULL) {
> -		struct page *page = NETFRONT_SKB_CB(skb)->page;
> -		void *vaddr = page_address(page);
> -		unsigned offset = NETFRONT_SKB_CB(skb)->offset;
> -
> -		memcpy(skb->data, vaddr + offset,
> -		       skb_headlen(skb));
> +		int pull_to = NETFRONT_SKB_CB(skb)->pull_to;
>  
> -		if (page != skb_frag_page(&skb_shinfo(skb)->frags[0]))
> -			__free_page(page);
> +		__pskb_pull_tail(skb, pull_to - skb_headlen(skb));
>  
>  		/* Ethernet work: Delayed to here as it peeks the header. */
>  		skb->protocol = eth_type_trans(skb, dev);
> @@ -913,7 +906,6 @@ static int xennet_poll(struct napi_struct *napi, int budget)
>  	struct sk_buff_head errq;
>  	struct sk_buff_head tmpq;
>  	unsigned long flags;
> -	unsigned int len;
>  	int err;
>  
>  	spin_lock(&np->rx_lock);
> @@ -955,24 +947,13 @@ err:
>  			}
>  		}
>  
> -		NETFRONT_SKB_CB(skb)->page =
> -			skb_frag_page(&skb_shinfo(skb)->frags[0]);
> -		NETFRONT_SKB_CB(skb)->offset = rx->offset;
> -
> -		len = rx->status;
> -		if (len > RX_COPY_THRESHOLD)
> -			len = RX_COPY_THRESHOLD;
> -		skb_put(skb, len);
> +		NETFRONT_SKB_CB(skb)->pull_to = rx->status;
> +		if (NETFRONT_SKB_CB(skb)->pull_to > RX_COPY_THRESHOLD)
> +			NETFRONT_SKB_CB(skb)->pull_to = RX_COPY_THRESHOLD;
>  
> -		if (rx->status > len) {
> -			skb_shinfo(skb)->frags[0].page_offset =
> -				rx->offset + len;
> -			skb_frag_size_set(&skb_shinfo(skb)->frags[0], rx->status - len);
> -			skb->data_len = rx->status - len;
> -		} else {
> -			__skb_fill_page_desc(skb, 0, NULL, 0, 0);
> -			skb_shinfo(skb)->nr_frags = 0;
> -		}
> +		skb_shinfo(skb)->frags[0].page_offset = rx->offset;
> +		skb_frag_size_set(&skb_shinfo(skb)->frags[0], rx->status);
> +		skb->data_len = rx->status;
>  
>  		i = xennet_fill_frags(np, skb, &tmpq);
>  
> @@ -999,7 +980,7 @@ err:
>  		 * receive throughout using the standard receive
>  		 * buffer size was cut by 25%(!!!).
>  		 */
> -		skb->truesize += skb->data_len - (RX_COPY_THRESHOLD - len);
> +		skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
>  		skb->len += skb->data_len;
>  
>  		if (rx->flags & XEN_NETRXF_csum_blank)
> -- 
> 1.7.2.5
> 
> 
> 
> --
> 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/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: First pass at MSG_FASTOPEN support in top-of-trunk netperf
From: H.K. Jerry Chu @ 2012-08-23 14:15 UTC (permalink / raw)
  To: Rick Jones; +Cc: netdev
In-Reply-To: <5020739A.3060809@hp.com>

Hi Rick,

On Mon, Aug 6, 2012 at 6:47 PM, Rick Jones <rick.jones2@hp.com> wrote:
> Folks -
>
> I have just checked-in to the top-of-trunk netperf
> (http://www.netperf.org/svn/netperf2/trunk) some changes which enable use of
> sendto(MSG_FASTOPEN) in a TCP_CRR test.  While I was checking the system
> calls I noticed that netperf was calling enable_enobufs() for every migrated
> test, not just a UDP_STREAM test (the one where it is needed), so I fixed
> that at the same time.  Baseline is taken with the fix in place.
>
> MSG_FASTOPEN is used when one adds a test-specific -F option to the netperf
> command line:
>
> netperf -t TCP_CRR -H <destination> -- -F
>
> Just testing the client side from a VM on my workstation (running a net-next
> pulled just before 16:30 Pacific time) to my workstation itself I notice the
> following:
>
> Without MSG_FASTOPEN:
> raj@tardy-ubuntu-1204:~/netperf2_trunk$ src/netperf -H tardy.usa.hp.com -t
> TCP_CRR -i 3,30
> MIGRATED TCP Connect/Request/Response TEST from 0.0.0.0 (0.0.0.0) port 0
> AF_INET to tardy.usa.hp.com () port 0 AF_INET : +/-2.500% @ 99% conf.  :
> demo
> Local /Remote
> Socket Size   Request  Resp.   Elapsed  Trans.
> Send   Recv   Size     Size    Time     Rate
> bytes  Bytes  bytes    bytes   secs.    per sec
>
> 16384  87380  1        1       10.00    2092.33
> 16384  87380
>
> With MSG_FASTOPEN
> raj@tardy-ubuntu-1204:~/netperf2_trunk$ src/netperf -H tardy.usa.hp.com -t
> TCP_CRR -i 3,30 -- -F
> MIGRATED TCP Connect/Request/Response TEST from 0.0.0.0 (0.0.0.0) port 0
> AF_INET to tardy.usa.hp.com () port 0 AF_INET : +/-2.500% @ 99% conf.  :
> demo
> !!! WARNING
> !!! Desired confidence was not achieved within the specified iterations.
> !!! This implies that there was variability in the test environment that
> !!! must be investigated before going further.
> !!! Confidence intervals: Throughput      : 6.565%
> !!!                       Local CPU util  : 0.000%
> !!!                       Remote CPU util : 0.000%
>
> Local /Remote
> Socket Size   Request  Resp.   Elapsed  Trans.
> Send   Recv   Size     Size    Time     Rate
> bytes  Bytes  bytes    bytes   secs.    per sec
>
> 16384  87380  1        1       10.00    2590.18
> 16384  87380
>
> There was a ~25% increase in TCP_CRR performance, even without the server
> actually accepting the magic TCP option.  Is that actually expected?

I have a locally enhanced netperf for TCP_CRR over Fast Open and I've
 noticed the numbers can change drastically between runs. I have not got
the time to investigate why. (Does it have to do with scheduler and CPU
locality?) How consistent is your perf number?

I plan to submit the server side code soon and will work with you to add
the server side support to TCP_CRR (it requires a new TCP_FASTOPEN
socket option to enable Fast Open on a listener.)

Thanks,

Jerry

> Admittedly, it eliminates a connect() call, but the sequence before was:
>
> socket()
> getsockopt(SO_SNDBUF)
> getsockopt(SO_RCVBUF)
> setsockopt(SO_REUSEADDR)
> bind()
> connect()
> sendto(4, "n", 1, 0, NULL, 0)           = 1
> recvfrom(4, "n", 1, 0, NULL, NULL)      = 1
> recvfrom(4, "", 1, 0, NULL, NULL)       = 0
> close(4)
>
> and with MSG_FASTOPEN only the connect() goes away.  Unless connect() is
> really heavy compared to what sendto(MSG_FASTOPEN) does or something else
> has changed wrt behaviour.
>
> Anyway, feel free to kick the tires on the netperf changes and let me know
> of any problems you encounter.
>
> happy benchmarking,
>
> rick jones
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net-next 04/13] e1000e: cleanup checkpatch PREFER_PR_LEVEL warning
From: Joe Perches @ 2012-08-23 14:01 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Bruce Allan, netdev, gospo, sassmann
In-Reply-To: <1345715813-20757-5-git-send-email-jeffrey.t.kirsher@intel.com>

On Thu, 2012-08-23 at 02:56 -0700, Jeff Kirsher wrote:
> From: Bruce Allan <bruce.w.allan@intel.com>
> 
> checkpatch warning: Prefer pr_info(... to printk(KERN_INFO, ...
[]
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
[]
> @@ -4330,9 +4330,8 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
>  	u32 ctrl = er32(CTRL);
>  
>  	/* Link status message must follow this format for user tools */
> -	printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
> -		adapter->netdev->name,
> -		adapter->link_speed,
> +	pr_info("e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
> +		adapter->netdev->name, adapter->link_speed,
>  		adapter->link_duplex == FULL_DUPLEX ? "Full" : "Half",
>  		(ctrl & E1000_CTRL_TFCE) && (ctrl & E1000_CTRL_RFCE) ? "Rx/Tx" :
>  		(ctrl & E1000_CTRL_RFCE) ? "Rx" :

I think these conversions are not a good idea.

When you have a specific message format that must be
followed, use printk.

pr_<level> may at some point in the near future use
#define pr_fmt(fmt) KBUiLD_MODNAME ": " fmt
as a global default equivalent.

^ permalink raw reply

* Re: [patch net-next 2/3] vlan: add helper which can be called to se it device is used by vlan
From: Jiri Pirko @ 2012-08-23 13:42 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: netdev, davem, brouer, Patrick McHardy
In-Reply-To: <50363112.4060407@mellanox.com>

Thu, Aug 23, 2012 at 03:33:06PM CEST, ogerlitz@mellanox.com wrote:
>On 23/08/2012 16:26, Jiri Pirko wrote:
>>also, remove unused vlan_info definition from header
>
>just a typo in the subject line, you probably wanted to say "see" and
>not "se"?

Yes. I need some coffe :(

>
>Or.
>
>>
>>CC: Patrick McHardy <kaber@trash.net>
>>Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>---
>>  include/linux/if_vlan.h | 9 +++++++--
>>  net/8021q/vlan_core.c   | 6 ++++++
>>  2 files changed, 13 insertions(+), 2 deletions(-)
>>
>>diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>>index a810987..e6ff12d 100644
>>--- a/include/linux/if_vlan.h
>>+++ b/include/linux/if_vlan.h
>>@@ -74,8 +74,6 @@ static inline struct vlan_ethhdr *vlan_eth_hdr(const struct sk_buff *skb)
>>  /* found in socket.c */
>>  extern void vlan_ioctl_set(int (*hook)(struct net *, void __user *));
>>-struct vlan_info;
>>-
>>  static inline int is_vlan_dev(struct net_device *dev)
>>  {
>>          return dev->priv_flags & IFF_802_1Q_VLAN;
>>@@ -101,6 +99,8 @@ extern int vlan_vids_add_by_dev(struct net_device *dev,
>>  				const struct net_device *by_dev);
>>  extern void vlan_vids_del_by_dev(struct net_device *dev,
>>  				 const struct net_device *by_dev);
>>+
>>+extern bool vlan_uses_dev(const struct net_device *dev);
>>  #else
>>  static inline struct net_device *
>>  __vlan_find_dev_deep(struct net_device *real_dev, u16 vlan_id)
>>@@ -151,6 +151,11 @@ static inline void vlan_vids_del_by_dev(struct net_device *dev,
>>  					const struct net_device *by_dev)
>>  {
>>  }
>>+
>>+static inline bool vlan_uses_dev(const struct net_device *dev)
>>+{
>>+	return false;
>>+}
>>  #endif
>>  /**
>>diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>>index d4babc6..517b940 100644
>>--- a/net/8021q/vlan_core.c
>>+++ b/net/8021q/vlan_core.c
>>@@ -372,3 +372,9 @@ void vlan_vids_del_by_dev(struct net_device *dev,
>>  		vlan_vid_del(dev, vid_info->vid);
>>  }
>>  EXPORT_SYMBOL(vlan_vids_del_by_dev);
>>+
>>+bool vlan_uses_dev(const struct net_device *dev)
>>+{
>>+	return rtnl_dereference(dev->vlan_info) ? true : false;
>>+}
>>+EXPORT_SYMBOL(vlan_uses_dev);
>

^ permalink raw reply

* Re: [patch net-next 2/3] vlan: add helper which can be called to se it device is used by vlan
From: Or Gerlitz @ 2012-08-23 13:33 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, brouer, Patrick McHardy
In-Reply-To: <1345728413-10298-3-git-send-email-jiri@resnulli.us>

On 23/08/2012 16:26, Jiri Pirko wrote:
> also, remove unused vlan_info definition from header

just a typo in the subject line, you probably wanted to say "see" and 
not "se"?

Or.

>
> CC: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>   include/linux/if_vlan.h | 9 +++++++--
>   net/8021q/vlan_core.c   | 6 ++++++
>   2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index a810987..e6ff12d 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -74,8 +74,6 @@ static inline struct vlan_ethhdr *vlan_eth_hdr(const struct sk_buff *skb)
>   /* found in socket.c */
>   extern void vlan_ioctl_set(int (*hook)(struct net *, void __user *));
>   
> -struct vlan_info;
> -
>   static inline int is_vlan_dev(struct net_device *dev)
>   {
>           return dev->priv_flags & IFF_802_1Q_VLAN;
> @@ -101,6 +99,8 @@ extern int vlan_vids_add_by_dev(struct net_device *dev,
>   				const struct net_device *by_dev);
>   extern void vlan_vids_del_by_dev(struct net_device *dev,
>   				 const struct net_device *by_dev);
> +
> +extern bool vlan_uses_dev(const struct net_device *dev);
>   #else
>   static inline struct net_device *
>   __vlan_find_dev_deep(struct net_device *real_dev, u16 vlan_id)
> @@ -151,6 +151,11 @@ static inline void vlan_vids_del_by_dev(struct net_device *dev,
>   					const struct net_device *by_dev)
>   {
>   }
> +
> +static inline bool vlan_uses_dev(const struct net_device *dev)
> +{
> +	return false;
> +}
>   #endif
>   
>   /**
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index d4babc6..517b940 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -372,3 +372,9 @@ void vlan_vids_del_by_dev(struct net_device *dev,
>   		vlan_vid_del(dev, vid_info->vid);
>   }
>   EXPORT_SYMBOL(vlan_vids_del_by_dev);
> +
> +bool vlan_uses_dev(const struct net_device *dev)
> +{
> +	return rtnl_dereference(dev->vlan_info) ? true : false;
> +}
> +EXPORT_SYMBOL(vlan_uses_dev);

^ permalink raw reply

* Re: [patch net-next 1/3] teamd: don't print warn message on -ESRCH during event send
From: Jiri Pirko @ 2012-08-23 13:31 UTC (permalink / raw)
  To: netdev; +Cc: davem, brouer, ogerlitz
In-Reply-To: <1345728413-10298-2-git-send-email-jiri@resnulli.us>

Oups. Subject should start with "team:" without "d" :(

David, should I repost?

Thu, Aug 23, 2012 at 03:26:51PM CEST, jiri@resnulli.us wrote:
>When no one is listening on NL socket, -ESRCH is returned and warning
>message is printed. This message is confusing people and in fact has no
>meaning. So do not print it in this case.
>
>Reported-by: Or Gerlitz <ogerlitz@mellanox.com>
>Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>---
> drivers/net/team/team.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index 17ce11b..4010bcd 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -2487,7 +2487,7 @@ static void __team_options_change_check(struct team *team)
> 			list_add_tail(&opt_inst->tmp_list, &sel_opt_inst_list);
> 	}
> 	err = team_nl_send_event_options_get(team, &sel_opt_inst_list);
>-	if (err)
>+	if (err && err != -ESRCH)
> 		netdev_warn(team->dev, "Failed to send options change via netlink (err %d)\n",
> 			    err);
> }
>@@ -2518,9 +2518,9 @@ static void __team_port_change_check(struct team_port *port, bool linkup)
> 
> send_event:
> 	err = team_nl_send_event_port_list_get(port->team);
>-	if (err)
>-		netdev_warn(port->team->dev, "Failed to send port change of device %s via netlink\n",
>-			    port->dev->name);
>+	if (err && err != -ESRCH)
>+		netdev_warn(port->team->dev, "Failed to send port change of device %s via netlink (err %d)\n",
>+			    port->dev->name, err);
> 
> }
> 
>-- 
>1.7.11.4
>

^ 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