netdev - Re: [PATCH 2/2] eHEA: Receive SKB Aggregation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070705082056.GA358@2ka.mipt.ru>
Date:	Thu, 5 Jul 2007 12:20:56 +0400
From:	Evgeniy Polyakov <johnpol@....mipt.ru>
To:	Jan-Bernd Themann <ossthema@...ibm.com>
Cc:	Jeff Garzik <jeff@...zik.org>, netdev <netdev@...r.kernel.org>,
	Christoph Raisch <raisch@...ibm.com>,
	Jan-Bernd Themann <themann@...ibm.com>,
	Marcus Eder <meder@...ibm.com>,
	Thomas Klein <tklein@...ibm.com>,
	Stefan Roscher <stefan.roscher@...ibm.com>
Subject: Re: [PATCH 2/2] eHEA: Receive SKB Aggregation

Hi Jan-Bernd.

[ Dropped spambot/i.e. unrelated mail lists ]

On Thu, Jul 05, 2007 at 09:26:30AM +0200, Jan-Bernd Themann (ossthema@...ibm.com) wrote:
> This patch enables the receive side processing to aggregate TCP packets within
> the HEA device driver. It analyses the packets already received after an
> interrupt arrived and forwards these as chains of SKBs for the same TCP
> connection with modified header field. We have seen a lower CPU load and
> improved throughput for small numbers of parallel TCP connections.
> As this feature is considered as experimental it is switched off by default
> and can be activated via a module parameter.

I've couple of comments on the driver, but mainly the fact of decreased
CPU usage itself - what was the magnitude of the win with this driver,
it looks like because of per-packet receive code path invocation is the
place of the latency...
Your implementation looks generic enough to be used by any driver, don't
you want to push it separately from eHEA driver?


> +static int lro_tcp_check(struct iphdr *iph, struct tcphdr *tcph,
> +			 int tcp_data_len, struct ehea_lro *lro)
> +{
> +	if (tcp_data_len == 0)
> +		return -1;
> +
> +	if (iph->ihl != IPH_LEN_WO_OPTIONS)
> +		return -1;
> +
> +	if (tcph->cwr || tcph->ece || tcph->urg || !tcph->ack || tcph->psh
> +	    || tcph->rst || tcph->syn || tcph->fin)
> +		return -1;
> +
> +	if (INET_ECN_is_ce(ipv4_get_dsfield(iph)))
> +		return -1;
> +
> +	if (tcph->doff != TCPH_LEN_WO_OPTIONS
> +	    && tcph->doff != TCPH_LEN_W_TIMESTAMP)
> +		return -1;
> +
> +	/* check tcp options (only timestamp allowed) */
> +	if (tcph->doff == TCPH_LEN_W_TIMESTAMP) {
> +		u32 *topt = (u32 *)(tcph + 1);
> +
> +		if (*topt != htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16)
> +				   | (TCPOPT_TIMESTAMP << 8) | TCPOLEN_TIMESTAMP))
> +			return -1;
> +
> +		/* timestamp should be in right order */
> +		topt++;
> +		if (lro && (ntohl(lro->tcp_rcv_tsval) > ntohl(*topt)))
> +			return -1;

This should use before/after technique like PAWS in TCP code or there will be a
problem with wrapper timestamps.

> +
> +		/* timestamp reply should not be zero */
> +		topt++;
> +		if (*topt == 0)
> +			return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void update_tcp_ip_header(struct ehea_lro *lro)
> +{
> +	struct iphdr *iph = lro->iph;
> +	struct tcphdr *tcph = lro->tcph;
> +	u32 *p;
> +
> +	tcph->ack_seq = lro->tcp_ack;
> +	tcph->window = lro->tcp_window;
> +
> +	if (lro->tcp_saw_tstamp) {
> +		p = (u32 *)(tcph + 1);
> +		*(p+2) = lro->tcp_rcv_tsecr;
> +	}
> +
> +	iph->tot_len = htons(lro->ip_tot_len);
> +	iph->check = 0;
> +	iph->check = ip_fast_csum((u8 *)lro->iph, iph->ihl);
> +}
> +
> +static void init_lro_desc(struct ehea_lro *lro, struct ehea_cqe *cqe,
> +			  struct sk_buff *skb, struct iphdr *iph,
> +			  struct tcphdr *tcph, u32 tcp_data_len)
> +{
> +	u32 *ptr;
> +
> +	lro->parent = skb;
> +	lro->iph = iph;
> +	lro->tcph = tcph;
> +	lro->tcp_next_seq = ntohl(tcph->seq) + tcp_data_len;
> +	lro->tcp_ack = ntohl(tcph->ack_seq);

How do you handle misordering or duplicate acks or resends?

> +	lro->skb_sg_cnt = 1;
> +	lro->ip_tot_len = ntohs(iph->tot_len);
> +
> +	if (tcph->doff == 8) {
> +		ptr = (u32 *)(tcph+1);
> +		lro->tcp_saw_tstamp = 1;
> +		lro->tcp_rcv_tsval = *(ptr+1);
> +		lro->tcp_rcv_tsecr = *(ptr+2);
> +	}
> +
> +	if (cqe->status & EHEA_CQE_VLAN_TAG_XTRACT) {
> +		lro->vlan_packet = 1;
> +		lro->vlan_tag = cqe->vlan_tag;
> +	}
> +
> +	lro->active = 1;
> +}

-- 
	Evgeniy Polyakov
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html