[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LFD.2.00.1010020129400.2462@ja.ssi.bg>
Date: Sat, 2 Oct 2010 01:35:28 +0300 (EEST)
From: Julian Anastasov <ja@....bg>
To: Simon Horman <horms@...ge.net.au>
cc: lvs-devel@...r.kernel.org, netdev@...r.kernel.org,
netfilter@...r.kernel.org, netfilter-devel@...r.kernel.org,
Jan Engelhardt <jengelh@...ozas.de>,
Stephen Hemminger <shemminger@...tta.com>,
Wensong Zhang <wensong@...ux-vs.org>,
Patrick McHardy <kaber@...sh.net>
Subject: Re: [patch v2 03/12] [PATCH 03/12] IPVS: compact
ip_vs_sched_persist()
Hello,
On Fri, 1 Oct 2010, Simon Horman wrote:
> Compact ip_vs_sched_persist() by setting up parameters
> and calling functions once.
>
> Signed-off-by: Simon Horman <horms@...ge.net.au>
> ---
>
> v2
> * Make "union nf_inet_addr fwmark" const
> * Don't remove the comment next to the declaration of dport
> * Add a comment to the declaration of vport
>
> Index: lvs-test-2.6/net/netfilter/ipvs/ip_vs_core.c
> ===================================================================
> --- lvs-test-2.6.orig/net/netfilter/ipvs/ip_vs_core.c 2010-10-01 21:56:39.000000000 +0900
> +++ lvs-test-2.6/net/netfilter/ipvs/ip_vs_core.c 2010-10-01 22:02:41.000000000 +0900
> @@ -193,10 +193,14 @@ ip_vs_sched_persist(struct ip_vs_service
> struct ip_vs_iphdr iph;
> struct ip_vs_dest *dest;
> struct ip_vs_conn *ct;
> - __be16 dport; /* destination port to forward */
> + 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;
>
> ip_vs_fill_iphdr(svc->af, skb_network_header(skb), &iph);
>
> @@ -227,119 +231,58 @@ ip_vs_sched_persist(struct ip_vs_service
> * service, and a template like <caddr, 0, vaddr, vport, daddr, dport>
> * is created for other persistent services.
> */
> - if (ports[1] == svc->port) {
> - /* Check if a template already exists */
> - if (svc->port != FTPPORT)
> - ct = ip_vs_ct_in_get(svc->af, iph.protocol, &snet, 0,
> - &iph.daddr, ports[1]);
> - else
> - ct = ip_vs_ct_in_get(svc->af, iph.protocol, &snet, 0,
> - &iph.daddr, 0);
> -
> - if (!ct || !ip_vs_check_template(ct)) {
> - /*
> - * No template found or the dest of the connection
> - * template is not available.
> - */
> - dest = svc->scheduler->schedule(svc, skb);
> - if (dest == NULL) {
> - IP_VS_DBG(1, "p-schedule: no dest found.\n");
> - return NULL;
> - }
> -
> - /*
> - * Create a template like <protocol,caddr,0,
> - * vaddr,vport,daddr,dport> for non-ftp service,
> - * and <protocol,caddr,0,vaddr,0,daddr,0>
> - * for ftp service.
> + {
> + if (ports[1] == svc->port) {
> + /* non-FTP template:
> + * <protocol, caddr, 0, vaddr, vport, daddr, dport>
> + * FTP template:
> + * <protocol, caddr, 0, vaddr, 0, daddr, 0>
> */
> if (svc->port != FTPPORT)
> - ct = ip_vs_conn_new(svc->af, iph.protocol,
> - &snet, 0,
> - &iph.daddr,
> - ports[1],
> - &dest->addr, dest->port,
> - IP_VS_CONN_F_TEMPLATE,
> - dest);
> - else
> - ct = ip_vs_conn_new(svc->af, iph.protocol,
> - &snet, 0,
> - &iph.daddr, 0,
> - &dest->addr, 0,
> - IP_VS_CONN_F_TEMPLATE,
> - dest);
> - if (ct == NULL)
> - return NULL;
> -
> - ct->timeout = svc->timeout;
> + vport = ports[1];
> } else {
> - /* set destination with the found template */
> - dest = ct->dest;
> - }
> - dport = dest->port;
> - } else {
> - /*
> - * Note: persistent fwmark-based services and persistent
> - * port zero service are handled here.
> - * fwmark template: <IPPROTO_IP,caddr,0,fwmark,0,daddr,0>
> - * port zero template: <protocol,caddr,0,vaddr,0,daddr,0>
> - */
> - if (svc->fwmark) {
> - union nf_inet_addr fwmark = {
> - .ip = htonl(svc->fwmark)
> - };
> -
> - ct = ip_vs_ct_in_get(svc->af, IPPROTO_IP, &snet, 0,
> - &fwmark, 0);
> - } else
> - ct = ip_vs_ct_in_get(svc->af, iph.protocol, &snet, 0,
> - &iph.daddr, 0);
> -
> - if (!ct || !ip_vs_check_template(ct)) {
> - /*
> - * If it is not persistent port zero, return NULL,
> - * otherwise create a connection template.
> + /* Note: persistent fwmark-based services and
> + * persistent port zero service are handled here.
> + * fwmark template:
> + * <IPPROTO_IP,caddr,0,fwmark,0,daddr,0>
> + * port zero template:
> + * <protocol,caddr,0,vaddr,0,daddr,0>
> */
> - if (svc->port)
> - return NULL;
> -
> - dest = svc->scheduler->schedule(svc, skb);
> - if (dest == NULL) {
> - IP_VS_DBG(1, "p-schedule: no dest found.\n");
> - return NULL;
> + if (svc->fwmark) {
> + protocol = IPPROTO_IP;
> + vaddr = &fwmark;
> }
> + }
> + }
>
> - /*
> - * Create a template according to the service
> - */
> - if (svc->fwmark) {
> - union nf_inet_addr fwmark = {
> - .ip = htonl(svc->fwmark)
> - };
> -
> - ct = ip_vs_conn_new(svc->af, IPPROTO_IP,
> - &snet, 0,
> - &fwmark, 0,
> - &dest->addr, 0,
> - IP_VS_CONN_F_TEMPLATE,
> - dest);
> - } else
> - ct = ip_vs_conn_new(svc->af, iph.protocol,
> - &snet, 0,
> - &iph.daddr, 0,
> - &dest->addr, 0,
> - IP_VS_CONN_F_TEMPLATE,
> - dest);
> - if (ct == NULL)
> - return NULL;
> + /* Check if a template already exists */
> + ct = ip_vs_ct_in_get(svc->af, protocol, &snet, 0, vaddr, vport);
>
> - ct->timeout = svc->timeout;
> - } else {
> - /* set destination with the found template */
> - dest = ct->dest;
> + if (!ct || !ip_vs_check_template(ct)) {
> + /* No template found or the dest of the connection
> + * template is not available.
> + */
> + dest = svc->scheduler->schedule(svc, skb);
> + if (!dest) {
> + IP_VS_DBG(1, "p-schedule: no dest found.\n");
> + return NULL;
> }
> - dport = ports[1];
> - }
> +
> + if (ports[1] == svc->port && svc->port != FTPPORT)
> + dport = dest->port;
> +
> + /* Create a template */
> + ct = ip_vs_conn_new(svc->af, protocol, &snet, 0,vaddr, vport,
> + &dest->addr, dport,
> + IP_VS_CONN_F_TEMPLATE, dest);
> + if (ct == NULL)
> + return NULL;
> +
> + ct->timeout = svc->timeout;
> + } else
> + /* set destination with the found template */
> + dest = ct->dest;
Here dport:
> + dport = dest->port;
should be:
dport = ports[1];
if (dport == svc->port && dest->port)
dport = dest->port;
> flags = (svc->flags & IP_VS_SVC_F_ONEPACKET
> && iph.protocol == IPPROTO_UDP)?
Regards
--
Julian Anastasov <ja@....bg>
--
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
Powered by blists - more mailing lists