netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Horman <horms@verge.net.au>
To: Julius Volz <juliusv@google.com>
Cc: netdev@vger.kernel.org, lvs-devel@vger.kernel.org,
	kaber@trash.net, vbusam@google.com,
	Sven Wegener <sven.wegener@stealer.net>
Subject: Re: [PATCH RFC 00/24] IPVS: Add first IPv6 support to IPVS
Date: Wed, 27 Aug 2008 17:17:15 +1000	[thread overview]
Message-ID: <20080827071714.GA30536@verge.net.au> (raw)
In-Reply-To: <20080821212948.GB4407@verge.net.au>

Here is a second round of thoughts after having gone through the whole series.


[PATCH RFC 06/24] IPVS: Add debug macros for v4 and v6 address output

* The #defines in ip_vs_dbg_addr seem a bit aquard.
  Could it be rearanged liks this?

static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len,
					const union nf_inet_addr *addr,
					int *idx)
{
       int len;
#ifdef CONFIG_IP_VS_IPV6
       if (af == AF_INET6)
	       len = snprintf(&buf[*idx], buf_len - *idx, "[" NIP6_FMT "]",
			      NIP6(addr->in6)) + 1;
       else
#endif
	       len = snprintf(&buf[*idx], buf_len - *idx, NIPQUAD_FMT,
			      NIPQUAD(addr->ip)) + 1;

       *idx += len;
       return &buf[*idx - len];
}

* The comment "/* Only use from within IP_VS_DBG_BUF() macro */"
  should also mention usage inside IP_VS_ERR_BUF()

* If IP_VS_DBG_ADDR() is used more than once inside a single
  IP_VS_DBG_BUF() or IP_VS_ERR_BUF() call, won't ip_vs_dbg_buf
  be set to the value one of the  calls to IP_VS_DBG_ADDR,
  thus overwriting other calls and producing incorrect debugging
  output?

[PATCH RFC 15/24] IPVS: Add support for IPv6 entry output in procfs files

* The netlink-aware ipvsadm code also seems to allow for dotted-quad
  representation of ipv4 addresses in proc. Is that representation used
  or planned to be used?

[PATCH RFC 17/24] IPVS: Make proc/net files output IPv6 entries

* It might be cleaner to do:

#ifdef CONFIG_IP_VS_IPV6
	if (cp->af == AF_INET6)
		seq_printf ...
	else
#endif
		seq_printf ...


General

* You need to reorder and or merge patches such that after each
  patch is applied the code will build and run. It is ok for
  a patch to add code which isn't used until a later patch is applied.

* Where possible please make lines <= 80 columns wide


  parent reply	other threads:[~2008-08-27  7:17 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-20 16:15 [PATCH RFC 00/24] IPVS: Add first IPv6 support to IPVS Julius Volz
2008-08-20 16:15 ` [PATCH RFC 01/24] IPVS: Add genetlink interface definitions to ip_vs.h Julius Volz
2008-08-20 16:15 ` [PATCH RFC 02/24] IPVS: Add genetlink interface implementation Julius Volz
2008-08-20 16:15 ` [PATCH RFC 03/24] IPVS: Add CONFIG_IP_VS_IPV6 option for IPv6 support Julius Volz
2008-08-20 16:15 ` [PATCH RFC 04/24] IPVS: Change IPVS data structures to support IPv6 addresses Julius Volz
2008-08-20 16:15 ` [PATCH RFC 05/24] IPVS: Add general v4/v6 helper functions / data structures Julius Volz
2008-08-20 16:15 ` [PATCH RFC 06/24] IPVS: Add debug macros for v4 and v6 address output Julius Volz
2008-08-20 16:15 ` [PATCH RFC 07/24] IPVS: Convert existing debug uses to use new macros Julius Volz
2008-08-20 16:15 ` [PATCH RFC 08/24] IPVS: Make protocol handler functions support IPv6 Julius Volz
2008-08-21 13:29   ` Brian Haley
2008-08-21 13:52     ` Julius Volz
2008-08-21 14:08       ` Brian Haley
2008-08-21 14:55         ` Julius Volz
2008-08-21 15:12           ` Brian Haley
2008-08-21 15:28             ` Julius Volz
2008-08-20 16:15 ` [PATCH RFC 09/24] IPVS: Add IPv6 Netfilter hooks and add/modify support functions Julius Volz
2008-08-20 16:15 ` [PATCH RFC 10/24] IPVS: Extend scheduling functions for IPv6 support Julius Volz
2008-08-27  6:28   ` Simon Horman
2008-08-27 16:02     ` Julius Volz
2008-08-20 16:15 ` [PATCH RFC 11/24] IPVS: Add IPv6 xmit functions Julius Volz
2008-08-20 16:15 ` [PATCH RFC 12/24] IPVS: Extend functions for getting/creating connections Julius Volz
2008-08-20 16:15 ` [PATCH RFC 13/24] IPVS: Add IPv6 support to ip_vs_conn_hashkey() Julius Volz
2008-08-20 16:15 ` [PATCH RFC 14/24] IPVS: Turn off FTP application helper for IPv6 Julius Volz
2008-08-20 16:15 ` [PATCH RFC 15/24] IPVS: Add support for IPv6 entry output in procfs files Julius Volz
2008-08-20 16:15 ` [PATCH RFC 16/24] IPVS: Add function to determine if IPv6 address is local Julius Volz
2008-08-20 16:15 ` [PATCH RFC 17/24] IPVS: Make proc/net files output IPv6 entries correctly Julius Volz
2008-08-20 16:15 ` [PATCH RFC 18/24] IPVS: Convert dest/service lookup functions Julius Volz
2008-08-20 16:15 ` [PATCH RFC 19/24] IPVS: Add IPv6 support flag to schedulers Julius Volz
2008-08-21  1:48   ` Simon Horman
2008-08-21 10:21     ` Julius Volz
2008-08-21 13:23       ` Simon Horman
2008-08-20 16:15 ` [PATCH RFC 20/24] IPVS: Add validity checks when adding/editing v6 services Julius Volz
2008-08-20 16:15 ` [PATCH RFC 21/24] IPVS: Only expose IPv4 entries through sockopt interface Julius Volz
2008-08-20 16:15 ` [PATCH RFC 22/24] IPVS: Add IPv6 support to genetlink interface Julius Volz
2008-08-20 16:15 ` [PATCH RFC 23/24] IPVS: Small address/af usage fixups Julius Volz
2008-08-20 16:15 ` [PATCH RFC 24/24] IPVS: Add notes about IPv6 changes Julius Volz
2008-08-21  1:17 ` [PATCH RFC 00/24] IPVS: Add first IPv6 support to IPVS Simon Horman
2008-08-21  9:59   ` Julius Volz
2008-08-21 21:29     ` Simon Horman
2008-08-21 22:01       ` Julius Volz
2008-08-27  5:59         ` Simon Horman
2008-08-21 22:05       ` Simon Horman
2008-08-22  9:56         ` Julius Volz
2008-08-22 10:05           ` Graeme Fowler
2008-08-22 10:49             ` Julius Volz
2008-08-22 11:23               ` Sven Wegener
2008-08-22 12:14                 ` Julius Volz
2008-08-23  9:20                   ` Graeme Fowler
2008-08-23 15:22                     ` Joseph Mack NA3T
2008-08-23 16:31                       ` Graeme Fowler
2008-08-24  2:13                         ` Joseph Mack NA3T
2008-08-23 23:07                       ` Julius Volz
2008-08-24  1:40                         ` Joseph Mack NA3T
2008-08-27  6:09                   ` Simon Horman
2008-08-27 15:24                     ` Julius Volz
2008-08-27 23:49                       ` Simon Horman
2008-08-27  7:17       ` Simon Horman [this message]
2008-08-27 15:09         ` Julius Volz
2008-08-27 23:48           ` Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080827071714.GA30536@verge.net.au \
    --to=horms@verge.net.au \
    --cc=juliusv@google.com \
    --cc=kaber@trash.net \
    --cc=lvs-devel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sven.wegener@stealer.net \
    --cc=vbusam@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).