[v2,04/12,04/12] IPVS: Add struct ip_vs_conn_param

From: Simon Horman <horms@verge.net.au>

On Fri, Oct 01, 2010 at 11:58:04PM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Fri, 1 Oct 2010, Simon Horman wrote:
> 
> >+static int
> >+ip_vs_conn_fill_param_proto(int af, const struct sk_buff *skb,
> >+			    const struct ip_vs_iphdr *iph,
> >+			    unsigned int proto_off, int inverse,
> >+			    struct ip_vs_conn_param *p)
> >+{
> >+	__be16 _ports[2], *pptr;
> >+
> >+	pptr = skb_header_pointer(skb, proto_off, sizeof(_ports), _ports);
> >+	if (pptr == NULL)
> >+		return 1;
> >+
> >+	if (likely(!inverse))
> >+		ip_vs_conn_fill_param(af, iph->protocol, &iph->saddr, pptr[0],
> >+				      &iph->daddr, pptr[1], p);
> >+	else
> 
> 	Next line is wrong for inverse=1, must be
> 	&iph->daddr, pptr[1], &iph->saddr, pptr[0]

Thanks, fixed.

> >+		ip_vs_conn_fill_param(af, iph->protocol, &iph->saddr, pptr[0],
> >+				      &iph->daddr, pptr[1], p);
> >+	return 0;
> >+}
> >+
> 
> 	May be comments before ip_vs_conn_out_get should be
> changed:
> 
> >@@ -341,9 +351,7 @@ struct ip_vs_conn *ip_vs_ct_in_get
> > *	s_addr, s_port: pkt source address (inside host)
> > *	d_addr, d_port: pkt dest address (foreign host)
> > */
> >-struct ip_vs_conn *ip_vs_conn_out_get
> >-(int af, int protocol, const union nf_inet_addr *s_addr, __be16 s_port,
> >- const union nf_inet_addr *d_addr, __be16 d_port)
> >+struct ip_vs_conn *ip_vs_conn_out_get(const struct ip_vs_conn_param *p)

I have updated it to:

/* Gets ip_vs_conn associated with supplied parameters in the
 * ip_vs_conn_tab.
 * Called for pkts coming from inside-to-OUTside.
 *      p->caddr, p->cport: pkt source address (inside host)
 *      p->vaddr, p->vport: pkt dest address (foreign host) */

> >===================================================================
> >--- lvs-test-2.6.orig/net/netfilter/ipvs/ip_vs_core.c	2010-10-01 22:06:23.000000000 +0900
> >+++ lvs-test-2.6/net/netfilter/ipvs/ip_vs_core.c	2010-10-01 22:10:46.000000000 +0900
> >@@ -193,14 +193,11 @@ ip_vs_sched_persist(struct ip_vs_service
> >	struct ip_vs_iphdr iph;
> >	struct ip_vs_dest *dest;
> >	struct ip_vs_conn *ct;
> >-	int protocol = iph.protocol;
> >	__be16 dport = 0;		/* destination port to forward */
> >-	__be16 vport = 0;		/* virtual service port */
> >	unsigned int flags;
> >	union nf_inet_addr snet;	/* source network of the client,
> >					   after masking */
> >-	const union nf_inet_addr fwmark = { .ip = htonl(svc->fwmark) };
> >-	const union nf_inet_addr *vaddr = &iph.daddr;
> >+	struct ip_vs_conn_param param;
> >
> >	ip_vs_fill_iphdr(svc->af, skb_network_header(skb), &iph);
> >
> >@@ -232,6 +229,11 @@ ip_vs_sched_persist(struct ip_vs_service
> >	 * is created for other persistent services.
> >	 */
> >	{
> >+		int protocol = iph.protocol;
> >+		const union nf_inet_addr *vaddr = &iph.daddr;
> >+		const union nf_inet_addr fwmark = { .ip = htonl(svc->fwmark) };
> >+		__be16 vport = 0;
> >+
> >		if (ports[1] == svc->port) {
> >			/* non-FTP template:
> >			 * <protocol, caddr, 0, vaddr, vport, daddr, dport>
> >@@ -253,11 +255,12 @@ ip_vs_sched_persist(struct ip_vs_service
> >				vaddr = &fwmark;
> >			}
> >		}
> >+		ip_vs_conn_fill_param(svc->af, protocol, &snet, 0,
> >+				      vaddr, vport, &param);
> >	}
> >
> >	/* Check if a template already exists */
> >-	ct = ip_vs_ct_in_get(svc->af, protocol, &snet, 0, vaddr, vport);
> >-
> >+	ct = ip_vs_ct_in_get(&param);
> >	if (!ct || !ip_vs_check_template(ct)) {
> >		/* No template found or the dest of the connection
> >		 * template is not available.
> >@@ -272,8 +275,7 @@ ip_vs_sched_persist(struct ip_vs_service
> >			dport = dest->port;
> >
> >		/* Create a template */
> >-		ct = ip_vs_conn_new(svc->af, protocol, &snet, 0,vaddr, vport,
> >-				    &dest->addr, dport,
> >+		ct = ip_vs_conn_new(&param, &dest->addr, dport,
> >				    IP_VS_CONN_F_TEMPLATE, dest);
> >		if (ct == NULL)
> >			return NULL;
> >@@ -291,12 +293,7 @@ ip_vs_sched_persist(struct ip_vs_service
> >	/*
> >	 *    Create a new connection according to the template
> >	 */
> 
> 	Missing ip_vs_conn_fill_param here?

Ooops, yes. I think that for some reason I thought it wasn't necessary.
I have added the following:

        ip_vs_conn_fill_param(svc->af, iph.protocol, &iph.saddr, ports[0],
                              &iph.daddr, ports[1], &param);

> >-	cp = ip_vs_conn_new(svc->af, iph.protocol,
> >-			    &iph.saddr, ports[0],
> >-			    &iph.daddr, ports[1],
> >-			    &dest->addr, dport,
> >-			    flags,
> >-			    dest);
> >+	cp = ip_vs_conn_new(&param, &dest->addr, dport, flags, dest);
> >	if (cp == NULL) {
> >		ip_vs_conn_put(ct);
> >		return NULL;
> 
> >===================================================================
> >--- lvs-test-2.6.orig/net/netfilter/ipvs/ip_vs_proto_ah_esp.c	2010-10-01 21:55:19.000000000 +0900
> >+++ lvs-test-2.6/net/netfilter/ipvs/ip_vs_proto_ah_esp.c	2010-10-01 22:23:33.000000000 +0900
> >@@ -40,6 +40,19 @@ struct isakmp_hdr {
> >
> >#define PORT_ISAKMP	500
> >
> >+static void
> >+ah_esp_conn_fill_param_proto(int af, const struct ip_vs_iphdr *iph,
> >+			     int inverse, struct ip_vs_conn_param *p)
> >+{
> >+	if (likely(!inverse))
> >+		ip_vs_conn_fill_param(af, IPPROTO_UDP,
> >+				      &iph->saddr, htons(PORT_ISAKMP),
> >+				      &iph->daddr, htons(PORT_ISAKMP), p);
> >+	else
> 
> 	For inverse=1 iph->protocol must be IPPROTO_UDP
> and &iph->daddr before &iph->saddr:

Thanks, fixed.

> >+		ip_vs_conn_fill_param(af, iph->protocol,
> >+				      &iph->saddr, htons(PORT_ISAKMP),
> >+				      &iph->daddr, htons(PORT_ISAKMP), p);
> >+}

I will repost the entire series after addressing the concerns
you raised with several of the other patches. But for reference
here is the revised version of this patch.

From 335c0ae2b64d8071762c50ea20b4f55bb12c2a5b Mon Sep 17 00:00:00 2001
From: Simon Horman <horms@verge.net.au>
Date: Sun, 22 Aug 2010 21:37:52 +0900
Subject: [patch v2.1 04/12] IPVS: Add struct ip_vs_conn_param

Signed-off-by: Simon Horman <horms@verge.net.au>
---

The motivation for this is to allow persistence engine modules to
fill in the parameters.

v0.3
* Add missing changes to ip_vs_ftp.c

v2
* make "union nf_inet_addr fwmark" const
* Update for the recent addition of ip_vs_nfct.c

v2.1
* As suggested by Julian Anastasov
  - Correct logic for inverse case in ip_vs_conn_fill_param_proto()
    and ah_esp_conn_fill_param_proto()
  - Update ip_vs_conn_out_get()'s comments as its parameters have changed
  - Add missing call to ip_vs_conn_fill_param() before the second
    invocation of ip_vs_conn_new() in ip_vs_sched_persist()

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

Message ID	20101002011350.GC2235@verge.net.au
State	Not Applicable, archived
Delegated to:	David Miller
Headers	show Return-Path: <netdev-owner@vger.kernel.org> X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id C3038B70AB for <patchwork-incoming@ozlabs.org>; Sat, 2 Oct 2010 11:14:06 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753172Ab0JBBN5 (ORCPT <rfc822;patchwork-incoming@ozlabs.org>); Fri, 1 Oct 2010 21:13:57 -0400 Received: from kirsty.vergenet.net ([202.4.237.240]:60591 "EHLO kirsty.vergenet.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751164Ab0JBBN4 (ORCPT <rfc822;netdev@vger.kernel.org>); Fri, 1 Oct 2010 21:13:56 -0400 Received: from ayumi.akashicho.tokyo.vergenet.net (219-109-213-121.bitcat.net [219.109.213.121]) by kirsty.vergenet.net (Postfix) with ESMTP id 90A2F24067; Sat, 2 Oct 2010 11:13:53 +1000 (EST) Received: by ayumi.akashicho.tokyo.vergenet.net (Postfix, from userid 7100) id DE651EDE0E5; Sat, 2 Oct 2010 10:13:50 +0900 (JST) Date: Sat, 2 Oct 2010 10:13:50 +0900 From: Simon Horman <horms@verge.net.au> To: Julian Anastasov <ja@ssi.bg> Cc: lvs-devel@vger.kernel.org, netdev@vger.kernel.org, netfilter@vger.kernel.org, netfilter-devel@vger.kernel.org, Jan Engelhardt <jengelh@medozas.de>, Stephen Hemminger <shemminger@vyatta.com>, Wensong Zhang <wensong@linux-vs.org>, Patrick McHardy <kaber@trash.net> Subject: Re: [patch v2 04/12] [PATCH 04/12] IPVS: Add struct ip_vs_conn_param Message-ID: <20101002011350.GC2235@verge.net.au> References: <20101001143517.645421976@akiko.akashicho.tokyo.vergenet.net> <20101001143942.035496196@akiko.akashicho.tokyo.vergenet.net> <alpine.LFD.2.00.1010012347290.2194@ja.ssi.bg> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <alpine.LFD.2.00.1010012347290.2194@ja.ssi.bg> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: <netdev.vger.kernel.org> X-Mailing-List: netdev@vger.kernel.org

[v2,04/12,04/12] IPVS: Add struct ip_vs_conn_param

Commit Message

Patch