Re: [PATCH next 00/84] ipvs: Stop guessing the network namespace (take 2)

2015-09-23 Thread Pablo Neira Ayuso 
On Wed, Sep 23, 2015 at 09:17:27AM +0900, Simon Horman wrote:
> On Tue, Sep 22, 2015 at 10:50:41AM +0200, Pablo Neira Ayuso wrote:
> > On Tue, Sep 22, 2015 at 10:22:13AM +0300, Julian Anastasov wrote:
> > [...]
> > > 
> > >   v2 looks good to me,
> > > 
> > > Acked-by: Julian Anastasov 
> > 
> > Thanks a lot for reviewing Julian.
> > 
> > Simon, please let me know how you want to handle this. Thanks.
> 
> I will see about taking it through my tree (as usual).

Thanks Simon!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH next 00/84] ipvs: Stop guessing the network namespace (take 2)

2015-09-22 Thread Simon Horman 
On Tue, Sep 22, 2015 at 10:50:41AM +0200, Pablo Neira Ayuso wrote:
> On Tue, Sep 22, 2015 at 10:22:13AM +0300, Julian Anastasov wrote:
> [...]
> > 
> > v2 looks good to me,
> > 
> > Acked-by: Julian Anastasov 
> 
> Thanks a lot for reviewing Julian.
> 
> Simon, please let me know how you want to handle this. Thanks.

I will see about taking it through my tree (as usual).
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH next 00/84] ipvs: Stop guessing the network namespace (take 2)

2015-09-22 Thread Julian Anastasov 

Hello,

On Mon, 21 Sep 2015, Eric W. Biederman wrote:

> I am gradually working my way through the netfilter stack passing struct
> down into the netfilter hooks and from the netfilter hooks and from
> there down into the functions that actually care.  This removes the need
> for netfilter functions to guess how to figure out how to compute which
> network namespace they are in and instead provides a simple and reliable
> method to do so.
> 
> The cleanups stand on their own but this is part of a larger effort to
> have routes with an output device that is not in the current network
> namespace.
> 
> The IPVS code has been a bit more of a challenge than most.  Just
> passing struct net through to where it is needed did not feel clean to
> me.  The practical issue is that the ipvs code in most places actually
> wants struct netns_ipvs and not struct net.
> 
> So as part of this process I have turned the relationship between struct
> net and the structs netns_ipvs, ip_vs_conn_param, ip_vs_conn, and
> ip_vs_service inside out.  I have modified the ipvs functions to take a
> struct netns_ipvs not a struct net.  The net is code with fewer
> conversions from one type of structure to another.  I did wind up adding
> a struct netns_ipvs parameter to quite a few functions that did not have
> it before so I could pass the structure down from the netfilter hooks to
> where it is actually needed to avoid guessing.
> 
> I have broken up the work in a bunch of small patches so there is at
> least a chance and reviewing that each step I took is correct.  The
> series compiles at each step so bisecting it should not be a problem
> if something weird comes up.
> 
> The first two changes in this series are actually bug fixes.  The first
> is a compile fix for a bug in sctp that came in, in the last round of
> ipvs changes merged into nf-next.  The second fixes an older bug where
> in pathological circumstances the wrong network namespace could be used
> when a proc file is written to.
> 
> The rest of the patchset is a bunch of boring changes getting pushing
> struct netns_ipvs (and by extension ipvs->net) where it needs to be.
> Either by replacing struct net pointers or adding new struct netns_ipvs
> pointers.  With a handful of other minor cleanups (like removing
> skb_net).
> 
> I have incorporated Julian Anastasov's feedback, which critically
> involves fixing a wrong piece of code.
> 
> The changes are also available against nf-next at:
> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.git master
> 
> My entire pending set of changes for those who want to look ahead is at:
> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.git 
> for-testing
> 
> Eric

v2 looks good to me,

Acked-by: Julian Anastasov 

Regards

--
Julian Anastasov 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH next 00/84] ipvs: Stop guessing the network namespace (take 2)

2015-09-22 Thread Pablo Neira Ayuso 
On Tue, Sep 22, 2015 at 10:22:13AM +0300, Julian Anastasov wrote:
[...]
> 
>   v2 looks good to me,
> 
> Acked-by: Julian Anastasov 

Thanks a lot for reviewing Julian.

Simon, please let me know how you want to handle this. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH next 00/84] ipvs: Stop guessing the network namespace (take 2)

2015-09-21 Thread Eric W. Biederman 

I am gradually working my way through the netfilter stack passing struct
down into the netfilter hooks and from the netfilter hooks and from
there down into the functions that actually care.  This removes the need
for netfilter functions to guess how to figure out how to compute which
network namespace they are in and instead provides a simple and reliable
method to do so.

The cleanups stand on their own but this is part of a larger effort to
have routes with an output device that is not in the current network
namespace.

The IPVS code has been a bit more of a challenge than most.  Just
passing struct net through to where it is needed did not feel clean to
me.  The practical issue is that the ipvs code in most places actually
wants struct netns_ipvs and not struct net.

So as part of this process I have turned the relationship between struct
net and the structs netns_ipvs, ip_vs_conn_param, ip_vs_conn, and
ip_vs_service inside out.  I have modified the ipvs functions to take a
struct netns_ipvs not a struct net.  The net is code with fewer
conversions from one type of structure to another.  I did wind up adding
a struct netns_ipvs parameter to quite a few functions that did not have
it before so I could pass the structure down from the netfilter hooks to
where it is actually needed to avoid guessing.

I have broken up the work in a bunch of small patches so there is at
least a chance and reviewing that each step I took is correct.  The
series compiles at each step so bisecting it should not be a problem
if something weird comes up.

The first two changes in this series are actually bug fixes.  The first
is a compile fix for a bug in sctp that came in, in the last round of
ipvs changes merged into nf-next.  The second fixes an older bug where
in pathological circumstances the wrong network namespace could be used
when a proc file is written to.

The rest of the patchset is a bunch of boring changes getting pushing
struct netns_ipvs (and by extension ipvs->net) where it needs to be.
Either by replacing struct net pointers or adding new struct netns_ipvs
pointers.  With a handful of other minor cleanups (like removing
skb_net).

I have incorporated Julian Anastasov's feedback, which critically
involves fixing a wrong piece of code.

The changes are also available against nf-next at:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.git master

My entire pending set of changes for those who want to look ahead is at:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.git for-testing

Eric

Eric W. Biederman (84):
  ipvs: Hoist computation of ipvs earlier in sctp_conn_schedule
  ipvs: Don't use current in proc_do_defense_mode
  ipvs: Use state->net in the ipvs forward functions
  ipvs: Store ipvs not net in struct ip_vs_conn
  ipvs: Store ipvs not net in struct ip_vs_conn_param
  ipvs: Pass ipvs not net to ip_vs_fill_conn
  ipvs: Store ipvs not net in struct ip_vs_service
  ipvs: Pass ipvs not net to ip_vs_svc_fwm_hashkey
  ipvs: Pass ipvs not net to __ip_vs_svc_fwm_find
  ipvs: Pass ipvs not net to ip_vs_svc_hashkey
  ipvs: Pass ipvs not net to __ip_vs_service_find
  ipvs: Pass ipvs not net to ip_vs_service_find
  ipvs: Pass ipvs not net to ip_vs_has_real_service
  ipvs: Pass ipvs not net to ip_vs_find_dest
  ipvs: Pass ipvs not net to ip_vs_trash_cleanup
  ipvs: Pass ipvs not net to __ip_vs_del_dest
  ipvs: Pass ipvs not net to ip_vs_dest_trash_expire
  ipvs: Cache ipvs in ip_vs_genl_set_cmd
  ipvs: Pass ipvs not net to ip_vs_add_service
  ipvs: Pass ipvs not net to ip_vs_flush
  ipvs: Pass ipvs not net to ip_vs_service_net_cleanup
  ipvs: Pass ipvs not net to ip_vs_zero_all
  ipvs: Cache ipvs in ip_vs_in_icmp and ip_vs_in_icmp_v6
  ipvs: Pass ipvs not net to ip_vs_proto_data_get
  ipvs: Pass ipvs not net to ip_vs_set_timeout
  ipvs: Pass ipvs not net to __ip_vs_get_service_entries
  ipvs: Pass ipvs not net to __ip_vs_get_dest_entries
  ipvs: Pass ipvs not net to __ip_vs_get_timeouts
  ipvs: Pass ipvs not net to ip_vs_genl_parse_service
  ipvs: Pass ipvs not net to ip_vs_genl_find_service
  ipvs: Pass ipvs not net to ip_vs_genl_new_daemon
  ipvs: Pass ipvs not net to ip_vs_genl_del_daemon
  ipvs: Pass ipvs not net to start_sync_thread
  ipvs: Pass ipvs not net to stop_sync_thread
  ipvs: Pass ipvs not net to make_send_sock
  ipvs: Pass ipvs not net to make_receive_sock
  ipvs: Store ipvs not net in struct ip_vs_sync_thread_data
  ipvs: Pass ipvs not net to ip_vs_process_message
  ipvs: Pass ipvs not net to ip_vs_sync_conn_v0
  ipvs: Pass ipvs not net to ip_vs_sync_conn
  ipvs: Pass ipvs not net to ip_vs_proc_conn
  ipvs: Pass ipvs not net to ip_vs_proc_sync_conn
  ipvs: Pass ipvs not net to ip_vs_sync_net_init
  ipvs: Pass ipvs not net to ip_vs_sync_net_cleanup
  ipvs: Pass ipvs not net to

[PATCH next 00/84] ipvs: Stop guessing the network namespace

2015-09-20 Thread Eric W. Biederman 

I am gradually working my way through the netfilter stack passing struct
down into the netfilter hooks and from the netfilter hooks and from
there down into the functions that actually care.  This removes the need
for netfilter functions to guess how to figure out how to compute which
network namespace they are in and instead provides a simple and reliable
method to do so.

The cleanups stand on their own but this is part of a larger effort
to have routes with an output device that is not in the current network
namespace.

The IPVS code has been a bit more of a challenge than most.  Just
passing struct net through to where it is needed did not feel clean
to me.  The practical issue is that the ipvs code in most places
actually wants struct netns_ipvs and not struct net.

So as part of this process I have turned the relationship between struct
net and the structs netns_ipvs, ip_vs_conn_param, ip_vs_conn, and
ip_vs_service inside out.  I have modified the ipvs functions to take a
struct netns_ipvs not a struct net.  The net is code with fewer
conversions from one type of structure to another.  I did wind up adding
a struct netns_ipvs parameter to quite a few functions that did not have
it before so I could pass the structure down from the netfilter hooks to
where it is actually needed to avoid guessing.

I have broken up the work in a bunch of small patches so there is at
least a chance and reviewing that each step I took is correct.  The
series compiles at each step so bisecting it should not be a problem
if something weird comes up.

The first two changes in this series are actually bug fixes.  The first
is a compile fix for a bug in sctp that came in, in the last round of
ipvs changes merged into nf-next.  The second fixes an older bug where
in pathological circumstances the wrong network namespace could be used
when a proc file is written to.

The rest of the patchset is a bunch of boring changes getting pushing
struct netns_ipvs (and by extension ipvs->net) where it needs to be.
Either by replacing struct net pointers or adding new struct netns_ipvs
pointers.  With a handful of other minor cleanups (like removing skb_net).

The changes are also available against nf-next at:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.git master

My entire pending set of changes for those who want to look ahead is at:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.git for-testing

Eric

Eric W. Biederman (84):
  ipvs: Hoist computation of ipvs earlier in sctp_conn_schedule
  ipvs: Don't use current in proc_do_defense_mode
  ipvs: Use state->net in the ipvs forward functions
  ipvs: Store ipvs not net in struct ip_vs_conn
  ipvs: Store ipvs not net in struct ip_vs_conn_param
  ipvs: Pass ipvs not net to ip_vs_fill_conn
  ipvs: Store ipvs not net in struct ip_vs_service
  ipvs: Pass ipvs not net to ip_vs_svc_fwm_hashkey
  ipvs: Pass ipvs not net to __ip_vs_svc_fwm_find
  ipvs: Pass ipvs not net to ip_vs_svc_hashkey
  ipvs: Pass ipvs not net to __ip_vs_service_find
  ipvs: Pass ipvs not net to ipvs_service_find
  ipvs: Pass ipvs not net to ip_vs_has_real_service
  ipvs: Pass ipvs not net to ip_vs_find_dest
  ipvs: Pass ipvs not net to ip_vs_trash_cleanup
  ipvs: Pass ipvs not net to __ip_vs_del_dest
  ipvs: Pass ipvs not net to ip_vs_dest_trash_expire
  ipvs: Cache ipvs in ip_vs_genl_set_cmd
  ipvs: Pass ipvs not net to ip_vs_add_service
  ipvs: Pass ipvs not net to ip_vs_flush
  ipvs: Pass ipvs not net to ip_vs_service_net_cleanup
  ipvs: Pass ipvs not net to ip_vs_zero_all
  ipvs: Cache ipvs in ip_vs_in_icmp and ip_vs_in_icmp_v6
  ipvs: Pass ipvs not net to ip_vs_proto_data_get
  ipvs: Pass ipvs not net to ip_vs_set_timeout
  ipvs: Pass ipvs not net to __ip_vs_get_servie_entries
  ipvs: Pass ipvs not net to __ip_vs_get_dest_entries
  ipvs: Pass ipvs not net to __ip_vs_get_timeouts
  ipvs: Pass ipvs not net to ip_vs_genl_parse_service
  ipvs: Pass ipvs not net to ip_vs_genl_find_service
  ipvs: Pass ipvs not net to ip_vs_genl_new_daemon
  ipvs: Pass ipvs not net to ip_vs_genl_del_daemon
  ipvs: Pass ipvs not net to start_sync_thread
  ipvs: Pass ipvs not net to stop_sync_thread
  ipvs: Pass ipvs not net to make_send_sock
  ipvs: Pass ipvs not net to make_receive_sock
  ipvs: Store ipvs not net in struct ip_vs_sync_thread_data
  ipvs: Pass ipvs not net to ip_vs_process_message
  ipvs: Pass ipvs not net to ip_vs_sync_conn_v0
  ipvs: Pass ipvs not net to ip_vs_sync_conn
  ipvs: Pass ipvs not net to ip_vs_proc_conn
  ipvs: Pass ipvs not net to ip_vs_proc_sync_conn
  ipvs: Pass ipvs not net to ip_vs_sync_net_init
  ipvs: Pass ipvs not net to ip_vs_sync_net_cleanup
  ipvs: Pass ipvs not net to ip_vs_genl_set_config
  ipvs: Pass ipvs not net to ip_vs_start_estimator aned ip_vs_stop_estimator
  ipvs: Pass

Re: [PATCH next 00/84] ipvs: Stop guessing the network namespace

2015-09-20 Thread Julian Anastasov 

Hello,

On Sun, 20 Sep 2015, Eric W. Biederman wrote:

> I am gradually working my way through the netfilter stack passing struct
> down into the netfilter hooks and from the netfilter hooks and from
> there down into the functions that actually care.  This removes the need
> for netfilter functions to guess how to figure out how to compute which
> network namespace they are in and instead provides a simple and reliable
> method to do so.
> 
> The cleanups stand on their own but this is part of a larger effort
> to have routes with an output device that is not in the current network
> namespace.
> 
> The IPVS code has been a bit more of a challenge than most.  Just
> passing struct net through to where it is needed did not feel clean
> to me.  The practical issue is that the ipvs code in most places
> actually wants struct netns_ipvs and not struct net.
> 
> So as part of this process I have turned the relationship between struct
> net and the structs netns_ipvs, ip_vs_conn_param, ip_vs_conn, and
> ip_vs_service inside out.  I have modified the ipvs functions to take a
> struct netns_ipvs not a struct net.  The net is code with fewer
> conversions from one type of structure to another.  I did wind up adding
> a struct netns_ipvs parameter to quite a few functions that did not have
> it before so I could pass the structure down from the netfilter hooks to
> where it is actually needed to avoid guessing.
> 
> I have broken up the work in a bunch of small patches so there is at
> least a chance and reviewing that each step I took is correct.  The
> series compiles at each step so bisecting it should not be a problem
> if something weird comes up.
> 
> The first two changes in this series are actually bug fixes.  The first
> is a compile fix for a bug in sctp that came in, in the last round of
> ipvs changes merged into nf-next.  The second fixes an older bug where
> in pathological circumstances the wrong network namespace could be used
> when a proc file is written to.
> 
> The rest of the patchset is a bunch of boring changes getting pushing
> struct netns_ipvs (and by extension ipvs->net) where it needs to be.
> Either by replacing struct net pointers or adding new struct netns_ipvs
> pointers.  With a handful of other minor cleanups (like removing skb_net).

I reviewed the patchset. Nice work, thanks!
Here are some comments:

01/84 ipvs: Hoist computation of ipvs earlier in sctp_conn_schedule

Simon had a fix for this problem, not sure what happened,
may be it was lost...

04/84 ipvs: Store ipvs not net in struct ip_vs_conn

Lost '!' here:

@@ -359,7 +359,7 @@ struct ip_vs_conn *ip_vs_ct_in_get(const struct
ip_vs_conn_param *p)
 
hlist_for_each_entry_rcu(cp, _vs_conn_tab[hash], c_list) {
if (unlikely(p->pe_data && p->pe->ct_match)) {
-   if (!ip_vs_conn_net_eq(cp, p->net))
+   if (net_eq(cp->ipvs->net, p->net))
continue;

Problem is then propagated to patch 05/84:

-   if (net_eq(cp->ipvs->net, p->net))
+   if (cp->ipvs == p->ipvs)

26/84 ipvs: Pass ipvs not net to __ip_vs_get_servie_entries

Missing 'c' in Subject

55/84 ipvs: Pass ipvs not net to register_ip_vs_app and 
unregister_ip_vs_app

Empty line after ipvs declaration:

 void __net_exit ip_vs_app_net_cleanup(struct net *net)
 {
-   unregister_ip_vs_app(net, NULL /* all */);
+   struct netns_ipvs *ipvs = net_ipvs(net);
+   unregister_ip_vs_app(ipvs, NULL /* all */);

here too:

 static void __ip_vs_ftp_exit(struct net *net)
 {
-   unregister_ip_vs_app(net, _vs_ftp);
+   struct netns_ipvs *ipvs = net_ipvs(net);
+   if (!ipvs)

61/84 ipvs: Pass ipvs into .conn_in_get and ip_vs_conn_in_get_proto

"dreive"

62/84 ipvs: Pass ipvs into conn_out_get

"dreive"

Also, scripts/checkpatch.pl --strict /tmp/*.patch gives
me warnings, sometimes for inherited syntax...

Regards

--
Julian Anastasov 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH next 00/84] ipvs: Stop guessing the network namespace

2015-09-20 Thread Eric W. Biederman 
Julian Anastasov  writes:

>   Hello,
>
>
>   I reviewed the patchset. Nice work, thanks!

Welcome.

> Here are some comments:
>
> 01/84 ipvs: Hoist computation of ipvs earlier in sctp_conn_schedule
>
>   Simon had a fix for this problem, not sure what happened,
> may be it was lost...
>
> 04/84 ipvs: Store ipvs not net in struct ip_vs_conn
>
>   Lost '!' here:

Very good catch thank you.

> @@ -359,7 +359,7 @@ struct ip_vs_conn *ip_vs_ct_in_get(const struct
> ip_vs_conn_param *p)
>  
> hlist_for_each_entry_rcu(cp, _vs_conn_tab[hash], c_list) {
> if (unlikely(p->pe_data && p->pe->ct_match)) {
> -   if (!ip_vs_conn_net_eq(cp, p->net))
> +   if (net_eq(cp->ipvs->net, p->net))
> continue;
>
>   Problem is then propagated to patch 05/84:
>
> -   if (net_eq(cp->ipvs->net, p->net))
> +   if (cp->ipvs == p->ipvs)
>
> 26/84 ipvs: Pass ipvs not net to __ip_vs_get_servie_entries
>
>   Missing 'c' in Subject
>
> 55/84 ipvs: Pass ipvs not net to register_ip_vs_app and 
> unregister_ip_vs_app
>
>   Empty line after ipvs declaration:
>
>  void __net_exit ip_vs_app_net_cleanup(struct net *net)
>  {
> -   unregister_ip_vs_app(net, NULL /* all */);
> +   struct netns_ipvs *ipvs = net_ipvs(net);
> +   unregister_ip_vs_app(ipvs, NULL /* all */);
>
>   here too:
>
>  static void __ip_vs_ftp_exit(struct net *net)
>  {
> -   unregister_ip_vs_app(net, _vs_ftp);
> +   struct netns_ipvs *ipvs = net_ipvs(net);
> +   if (!ipvs)
>
> 61/84 ipvs: Pass ipvs into .conn_in_get and ip_vs_conn_in_get_proto
>
>   "dreive"
>
> 62/84 ipvs: Pass ipvs into conn_out_get
>
>   "dreive"

I have fixed the above, and pushed the changes into my git tree.
I will resend the patchset tomorrow if nothing else shows up.

>   Also, scripts/checkpatch.pl --strict /tmp/*.patch gives
> me warnings, sometimes for inherited syntax...

I looked and I have fixed one or two of those, but mostly inherited
syntax and lines too long I am not tocuhing.  The odds of introducing
or obscuring an error right now far outweigh the odds of making
something better.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html