All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luciano Coelho <luciano.coelho@Nokia.com>
To: ext Jan Engelhardt <jengelh@medozas.de>
Cc: "netfilter-devel@vger.kernel.org"
	<netfilter-devel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"kaber@trash.net" <kaber@trash.net>,
	"sameo@linux.intel.com" <sameo@linux.intel.com>
Subject: Re: [RFC v2] netfilter: xt_condition: add condition target
Date: Tue, 20 Jul 2010 14:04:15 +0300	[thread overview]
Message-ID: <1279623855.16431.4.camel@powerslave> (raw)
In-Reply-To: <alpine.LSU.2.01.1007201235250.20447@obet.zrqbmnf.qr>

On Tue, 2010-07-20 at 12:45 +0200, ext Jan Engelhardt wrote:
> On Tuesday 2010-07-20 11:50, Luciano Coelho wrote:
> > struct xt_condition_mtinfo {
> >-	char name[31];
> >+	char name[XT_CONDITION_MAX_NAME_SIZE];
> > 	__u8 invert;
> > 
> > 	/* Used internally by the kernel */
> > 	void *condvar __attribute__((aligned(8)));
> > };
> > 
> >+struct condition_tg_info {
> 
> In the line of standardized naming, xt_condition_tginfo.

Ack.


> >+	char name[XT_CONDITION_MAX_NAME_SIZE];
> >+	__u8 enabled;
> 
> No u32 yet?

Yes, I decided to make this in different steps.  I'll be submitting a
new patch with the u32 (and the binary operators support) pretty soon.


> >+static struct xt_target condition_tg_reg __read_mostly = {
> >+       .name           = "CONDITION",
> >+       .family         = NFPROTO_UNSPEC,
> >+       .target         = condition_tg_target,
> >+       .targetsize     = sizeof(struct condition_tg_info),
> >+       .checkentry     = condition_tg_checkentry,
> >+       .destroy        = condition_tg_destroy,
> >+       .me             = THIS_MODULE,
> >+};
> >+
> > static struct xt_match condition_mt_reg __read_mostly = {
> >        .name       = "condition",
> >        .revision   = 1,
> 
> (I see that you just sent a diff from the previous submission. That
> in itself is ok.) Since xt_condition is a new module to upstream but
> already exists in Xtables-addons, it makes sense to use a
> .revision number of 2 for the initial Linux kernel submission,
> also because the struct contents are different from those currently
> in Xt-a.

Yes, I made this patch on top of the one you have sent earlier for
upstream inclusion.  There were some comments from Patrick to that one
and, as I said in my email yesterday, I'll rebase the target patches
once the original one is included upstream.

Do you want me to take a look at Patrick's comments and resubmit the
patch you've sent with the changes Patrick asked for?

I'll change the revision to 2 as well.


> From an overall quick glance, looks good!

Thanks for your review and help on this!

-- 
Cheers,
Luca.


  reply	other threads:[~2010-07-20 11:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-20  9:50 [RFC v2] netfilter: xt_condition: add condition target Luciano Coelho
2010-07-20 10:45 ` Jan Engelhardt
2010-07-20 11:04   ` Luciano Coelho [this message]
2010-07-20 11:11     ` Jan Engelhardt
2010-07-20 11:32       ` Luciano Coelho
2010-07-20 11:46         ` Jan Engelhardt
2010-07-20 11:53           ` Luciano Coelho

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=1279623855.16431.4.camel@powerslave \
    --to=luciano.coelho@nokia.com \
    --cc=jengelh@medozas.de \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=sameo@linux.intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.