All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Patrick McHardy <kaber@trash.net>
Cc: Hans Schillstrom <hans@schillstrom.com>,
	netdev@vger.kernel.org, lvs-devel@vger.kernel.org,
	Julian Anastasov <ja@ssi.bg>, Simon Horman <horms@verge.net.au>,
	Wensong Zhang <wensong@linux-vs.org>,
	netfilter-devel@vger.kernel.org
Subject: Re: Re[3]: [PATCH 2/3] ipvs: Fix faulty IPv6 extension header  handling in IPVS
Date: Wed, 29 Aug 2012 11:37:14 +0200	[thread overview]
Message-ID: <1346233034.3069.450.camel@localhost> (raw)
In-Reply-To: <Pine.GSO.4.63.1208271401140.17922@stinky-local.trash.net>

To Hans and Patrick,

On Mon, 2012-08-27 at 14:02 +0200, Patrick McHardy wrote:
> On Mon, 27 Aug 2012, Hans Schillstrom wrote:
> 
> >>>>
> >>>> On Mon, 20 Aug 2012, Jesper Dangaard Brouer wrote:
> >>>>
> >>>>> Based on patch from: Hans Schillstrom
> >>>>>
> >>>>> IPv6 headers must be processed in order of appearance,
> >>>>> neither can it be assumed that Upper layer headers is first.
> >>>>> If anything else than L4 is the first header IPVS will throw it.
> >>>>>
> >>>>> IPVS will write SNAT & DNAT modifications at a fixed pos which
> >>>>> will corrupt the message. Proper header position must be found
> >>>>> before writing modifying packet.
> >>>>>
> >>>>> This patch contains a lot of API changes.  This is done, to avoid
> >>>>> the costly scan of finding the IPv6 headers, via ipv6_find_hdr().
> >>>>> Finding the IPv6 headers is done as early as possible, and passed
> >>>>> on as a pointer "struct ip_vs_iphdr *" to the affected functions.
> >>>>
> >>>> How about we change netfilter to set up the skb's transport header
> >>>> at an early time so we can avoid all (most of) these header scans
> >>>> in netfilter?
> >>>
> >>> I think that would be great, maybe it should be global i.e. not only a netfilter issue.
> >>
> >> I think in most other cases the headers are supposed to be processed
> >> sequentially. One problem though - to be useful for netfilter/IPVS
> >> we'd also need to store the transport layer protocol somewhere.
> >
> > I guess that's the problem, adding it to the skb will not be popular ....
> > Right now I don't have a good solution, maybe a more generic netfilter ptr in the skb ...
> 
> I guess inet6_skb_parm will be at least slightly more popular than
> adding it to the skb itself. The netfilter pointers are all used for
> optional things, so we can't really add it to any of those.

Okay, but how do we go from here?

Hans, should this hold back the patch ("ipvs: Fix faulty IPv6 extension
header handling in IPVS").  Or should we pursue our patch, and circle
back later once e.g. Patrick have found a generic solution for IPv6
transport header handling?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer



WARNING: multiple messages have this Message-ID (diff)
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Patrick McHardy <kaber@trash.net>
Cc: Hans Schillstrom <hans@schillstrom.com>,
	netdev@vger.kernel.org, lvs-devel@vger.kernel.org,
	Julian Anastasov <ja@ssi.bg>, Simon Horman <horms@verge.net.au>,
	Wensong Zhang <wensong@linux-vs.org>,
	netfilter-devel@vger.kernel.org
Subject: Re: Re[3]: [PATCH 2/3] ipvs: Fix faulty IPv6 extension header handling in IPVS
Date: Wed, 29 Aug 2012 11:37:14 +0200	[thread overview]
Message-ID: <1346233034.3069.450.camel@localhost> (raw)
In-Reply-To: <Pine.GSO.4.63.1208271401140.17922@stinky-local.trash.net>

To Hans and Patrick,

On Mon, 2012-08-27 at 14:02 +0200, Patrick McHardy wrote:
> On Mon, 27 Aug 2012, Hans Schillstrom wrote:
> 
> >>>>
> >>>> On Mon, 20 Aug 2012, Jesper Dangaard Brouer wrote:
> >>>>
> >>>>> Based on patch from: Hans Schillstrom
> >>>>>
> >>>>> IPv6 headers must be processed in order of appearance,
> >>>>> neither can it be assumed that Upper layer headers is first.
> >>>>> If anything else than L4 is the first header IPVS will throw it.
> >>>>>
> >>>>> IPVS will write SNAT & DNAT modifications at a fixed pos which
> >>>>> will corrupt the message. Proper header position must be found
> >>>>> before writing modifying packet.
> >>>>>
> >>>>> This patch contains a lot of API changes.  This is done, to avoid
> >>>>> the costly scan of finding the IPv6 headers, via ipv6_find_hdr().
> >>>>> Finding the IPv6 headers is done as early as possible, and passed
> >>>>> on as a pointer "struct ip_vs_iphdr *" to the affected functions.
> >>>>
> >>>> How about we change netfilter to set up the skb's transport header
> >>>> at an early time so we can avoid all (most of) these header scans
> >>>> in netfilter?
> >>>
> >>> I think that would be great, maybe it should be global i.e. not only a netfilter issue.
> >>
> >> I think in most other cases the headers are supposed to be processed
> >> sequentially. One problem though - to be useful for netfilter/IPVS
> >> we'd also need to store the transport layer protocol somewhere.
> >
> > I guess that's the problem, adding it to the skb will not be popular ....
> > Right now I don't have a good solution, maybe a more generic netfilter ptr in the skb ...
> 
> I guess inet6_skb_parm will be at least slightly more popular than
> adding it to the skb itself. The netfilter pointers are all used for
> optional things, so we can't really add it to any of those.

Okay, but how do we go from here?

Hans, should this hold back the patch ("ipvs: Fix faulty IPv6 extension
header handling in IPVS").  Or should we pursue our patch, and circle
back later once e.g. Patrick have found a generic solution for IPv6
transport header handling?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer



  reply	other threads:[~2012-08-29  9:37 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-27 11:58 Re[3]: [PATCH 2/3] ipvs: Fix faulty IPv6 extension header handling in IPVS Hans Schillstrom
2012-08-27 11:58 ` Hans Schillstrom
2012-08-27 12:02 ` Patrick McHardy
2012-08-29  9:37   ` Jesper Dangaard Brouer [this message]
2012-08-29  9:37     ` Jesper Dangaard Brouer
2012-08-29 12:28     ` Patrick McHardy
2012-08-29 12:28       ` Patrick McHardy
2012-09-03 21:15       ` [RFC PATCH 0/2] RFC: Caching IPv6 exthdr in skb->cb[] Jesper Dangaard Brouer
2012-09-03 21:16         ` [RFC PATCH 1/2] net: Cache IPv6 extension header "skip" parsing Jesper Dangaard Brouer
2012-09-03 21:16         ` [RFC PATCH 2/2] netfilter: More users of ipv6_find_hdr_cb() Jesper Dangaard Brouer
2012-09-04  7:26         ` [RFC PATCH 0/2] RFC: Caching IPv6 exthdr in skb->cb[] Julian Anastasov

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=1346233034.3069.450.camel@localhost \
    --to=brouer@redhat.com \
    --cc=hans@schillstrom.com \
    --cc=horms@verge.net.au \
    --cc=ja@ssi.bg \
    --cc=kaber@trash.net \
    --cc=lvs-devel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=wensong@linux-vs.org \
    /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.