* [RFC davem] revert: net: Make skb->skb_iif always track skb->dev @ 2013-01-12 13:48 Oliver Hartkopp 2013-01-12 18:13 ` Jiri Pirko 2013-01-12 21:23 ` David Miller 0 siblings, 2 replies; 24+ messages in thread From: Oliver Hartkopp @ 2013-01-12 13:48 UTC (permalink / raw) To: David Miller; +Cc: Linux Netdev List Hello Dave, in your below patch from 23 Jul 2012 you removed the check for an already set value of skb_iif in net/core/dev.c I'm currently working on a solution to prevent some routed CAN frames to be sent back onto the originating network device. With your patch it is not possible anymore to check on which netdev the CAN frame has originally been received, as for every routing the frame goes through netif_receive_skb(), which hard sets skb->skb_iif = skb->dev->ifindex and therefore kills the original incoming interface index. To me it is not clear why skb_iff is needed anyway as the value should always be available via skb->dev->ifindex, right? But if skb_iff has any right to exist it should contain the first incoming interface on the host IMO. Please correct my if i'm wrong and/or tell me what your commit message means in respect to my request and why skb->dev->ifindex is not used instead of skb_iif. I feel somehow lost about the skb_iif intention ... Best regards, Oliver --- http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commitdiff;h=b68581778cd0051a3fb9a2b614dee7eccb5127ff net: Make skb->skb_iif always track skb->dev Make it follow device decapsulation, from things such as VLAN and bonding. The stuff that actually cares about pre-demuxed device pointers, is handled by the "orig_dev" variable in __netif_receive_skb(). And the only consumer of that is the po->origdev feature of AF_PACKET sockets. Signed-off-by: David S. Miller <davem@davemloft.net> --- diff --git a/net/core/dev.c b/net/core/dev.c index cca02ae..0ebaea1 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3173,8 +3173,6 @@ static int __netif_receive_skb(struct sk_buff *skb) if (netpoll_receive_skb(skb)) return NET_RX_DROP; - if (!skb->skb_iif) - skb->skb_iif = skb->dev->ifindex; orig_dev = skb->dev; skb_reset_network_header(skb); @@ -3186,6 +3184,7 @@ static int __netif_receive_skb(struct sk_buff *skb) rcu_read_lock(); another_round: + skb->skb_iif = skb->dev->ifindex; __this_cpu_inc(softnet_data.processed); ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-12 13:48 [RFC davem] revert: net: Make skb->skb_iif always track skb->dev Oliver Hartkopp @ 2013-01-12 18:13 ` Jiri Pirko 2013-01-12 18:40 ` Oliver Hartkopp 2013-01-12 21:23 ` David Miller 1 sibling, 1 reply; 24+ messages in thread From: Jiri Pirko @ 2013-01-12 18:13 UTC (permalink / raw) To: Oliver Hartkopp; +Cc: David Miller, Linux Netdev List Sat, Jan 12, 2013 at 02:48:14PM CET, socketcan@hartkopp.net wrote: >Hello Dave, > >in your below patch from 23 Jul 2012 you removed the check for an already set >value of skb_iif in net/core/dev.c > >I'm currently working on a solution to prevent some routed CAN frames to be >sent back onto the originating network device. Hm, I'm not sure where exactly you want to use this information, but can_rcv() can get it from orig_dev->ifindex > >With your patch it is not possible anymore to check on which netdev the >CAN frame has originally been received, as for every routing the frame >goes through netif_receive_skb(), which hard sets > > skb->skb_iif = skb->dev->ifindex > >and therefore kills the original incoming interface index. > >To me it is not clear why skb_iff is needed anyway as the value should >always be available via skb->dev->ifindex, right? > >But if skb_iff has any right to exist it should contain the first incoming >interface on the host IMO. > >Please correct my if i'm wrong and/or tell me what your commit message means >in respect to my request and why skb->dev->ifindex is not used instead of >skb_iif. I feel somehow lost about the skb_iif intention ... I believe that skb_iif should be removed. > >Best regards, >Oliver > >--- > > >http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commitdiff;h=b68581778cd0051a3fb9a2b614dee7eccb5127ff > >net: Make skb->skb_iif always track skb->dev > >Make it follow device decapsulation, from things such as VLAN and >bonding. > >The stuff that actually cares about pre-demuxed device pointers, is >handled by the "orig_dev" variable in __netif_receive_skb(). And >the only consumer of that is the po->origdev feature of AF_PACKET >sockets. > >Signed-off-by: David S. Miller <davem@davemloft.net> >--- > >diff --git a/net/core/dev.c b/net/core/dev.c >index cca02ae..0ebaea1 100644 >--- a/net/core/dev.c >+++ b/net/core/dev.c >@@ -3173,8 +3173,6 @@ static int __netif_receive_skb(struct sk_buff *skb) > if (netpoll_receive_skb(skb)) > return NET_RX_DROP; > >- if (!skb->skb_iif) >- skb->skb_iif = skb->dev->ifindex; > orig_dev = skb->dev; > > skb_reset_network_header(skb); >@@ -3186,6 +3184,7 @@ static int __netif_receive_skb(struct sk_buff *skb) > rcu_read_lock(); > > another_round: >+ skb->skb_iif = skb->dev->ifindex; > > __this_cpu_inc(softnet_data.processed); > > > >-- >To unsubscribe from this list: send the line "unsubscribe netdev" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-12 18:13 ` Jiri Pirko @ 2013-01-12 18:40 ` Oliver Hartkopp 2013-01-12 19:37 ` Jiri Pirko 0 siblings, 1 reply; 24+ messages in thread From: Oliver Hartkopp @ 2013-01-12 18:40 UTC (permalink / raw) To: Jiri Pirko; +Cc: David Miller, Linux Netdev List Hello Jiri, On 12.01.2013 19:13, Jiri Pirko wrote: > Sat, Jan 12, 2013 at 02:48:14PM CET, socketcan@hartkopp.net wrote: >> Hello Dave, >> >> in your below patch from 23 Jul 2012 you removed the check for an already set >> value of skb_iif in net/core/dev.c >> >> I'm currently working on a solution to prevent some routed CAN frames to be >> sent back onto the originating network device. > > Hm, I'm not sure where exactly you want to use this information, but > can_rcv() can get it from orig_dev->ifindex No - it's not in can_rcv() ... Depending on the filter lists in can_rcv() the received skb is passed to the function can_can_gw_rcv() in net/can/gw.c. An there i wanted to add this code to omit sending the CAN frame on the originating interface: @@ .. @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data if (!(gwj->dst.dev->flags & IFF_UP)) { gwj->dropped_frames++; return; } + /* is sending the skb back to the incoming interface allowed? */ + if (!(gwj->flags & CGW_FLAGS_CAN_IIF_TX_OK) && + skb->skb_iif == gwj->dst.dev->ifindex) + return; + /* * clone the given skb, which has not been done in can_rcv() * This works fine, when the patch from Dave is reverted. I did not find any good solution to preserve the originating netdev over several netif_receive_skb() calls - but this skb_iif which has been made unusable ... (..) >> >> To me it is not clear why skb_iff is needed anyway as the value should >> always be available via skb->dev->ifindex, right? >> >> But if skb_iff has any right to exist it should contain the first incoming >> interface on the host IMO. >> >> Please correct my if i'm wrong and/or tell me what your commit message means >> in respect to my request and why skb->dev->ifindex is not used instead of >> skb_iif. I feel somehow lost about the skb_iif intention ... > > I believe that skb_iif should be removed. AFAICS skb->skb_iif is used as some kind of cached variable for the original skb->dev->ifindex - or is it really possible that skb->skb_iif is set while skb->dev->ifindex is not accessible? Btw. i my case skb_iif would make sense though. Regards, Oliver >> >> --- >> >> >> http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commitdiff;h=b68581778cd0051a3fb9a2b614dee7eccb5127ff >> >> net: Make skb->skb_iif always track skb->dev >> >> Make it follow device decapsulation, from things such as VLAN and >> bonding. >> >> The stuff that actually cares about pre-demuxed device pointers, is >> handled by the "orig_dev" variable in __netif_receive_skb(). And >> the only consumer of that is the po->origdev feature of AF_PACKET >> sockets. >> >> Signed-off-by: David S. Miller <davem@davemloft.net> >> --- >> >> diff --git a/net/core/dev.c b/net/core/dev.c >> index cca02ae..0ebaea1 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -3173,8 +3173,6 @@ static int __netif_receive_skb(struct sk_buff *skb) >> if (netpoll_receive_skb(skb)) >> return NET_RX_DROP; >> >> - if (!skb->skb_iif) >> - skb->skb_iif = skb->dev->ifindex; >> orig_dev = skb->dev; >> >> skb_reset_network_header(skb); >> @@ -3186,6 +3184,7 @@ static int __netif_receive_skb(struct sk_buff *skb) >> rcu_read_lock(); >> >> another_round: >> + skb->skb_iif = skb->dev->ifindex; >> >> __this_cpu_inc(softnet_data.processed); >> >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-12 18:40 ` Oliver Hartkopp @ 2013-01-12 19:37 ` Jiri Pirko 2013-01-12 20:14 ` Oliver Hartkopp 0 siblings, 1 reply; 24+ messages in thread From: Jiri Pirko @ 2013-01-12 19:37 UTC (permalink / raw) To: Oliver Hartkopp; +Cc: David Miller, Linux Netdev List Sat, Jan 12, 2013 at 07:40:33PM CET, socketcan@hartkopp.net wrote: >Hello Jiri, > >On 12.01.2013 19:13, Jiri Pirko wrote: > >> Sat, Jan 12, 2013 at 02:48:14PM CET, socketcan@hartkopp.net wrote: >>> Hello Dave, >>> >>> in your below patch from 23 Jul 2012 you removed the check for an already set >>> value of skb_iif in net/core/dev.c >>> >>> I'm currently working on a solution to prevent some routed CAN frames to be >>> sent back onto the originating network device. >> >> Hm, I'm not sure where exactly you want to use this information, but >> can_rcv() can get it from orig_dev->ifindex > > >No - it's not in can_rcv() ... > >Depending on the filter lists in can_rcv() the received skb is passed to the >function can_can_gw_rcv() in net/can/gw.c. > >An there i wanted to add this code to omit sending the CAN frame on the >originating interface: > >@@ .. @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data > > if (!(gwj->dst.dev->flags & IFF_UP)) { > gwj->dropped_frames++; > return; > } > >+ /* is sending the skb back to the incoming interface allowed? */ >+ if (!(gwj->flags & CGW_FLAGS_CAN_IIF_TX_OK) && >+ skb->skb_iif == gwj->dst.dev->ifindex) >+ return; >+ > /* > * clone the given skb, which has not been done in can_rcv() > * > >This works fine, when the patch from Dave is reverted. > >I did not find any good solution to preserve the originating netdev over >several netif_receive_skb() calls - but this skb_iif which has been made >unusable ... Well, look at struct receiver in net/can/af_can.h: struct receiver { struct hlist_node list; struct rcu_head rcu; canid_t can_id; canid_t mask; unsigned long matches; void (*func)(struct sk_buff *, void *); void *data; char *ident; }; your can_can_gw_rcv() is callback registered as ->func here. This ->func is called from chain can_rcv->can_receive->can_rcv_filter->deliver So just extend ->func to something like: void (*func)(struct sk_buff *, struct neti_device *orig_dev, void *); and pass the orig_dev all the way through the chain. > >(..) > >>> >>> To me it is not clear why skb_iff is needed anyway as the value should >>> always be available via skb->dev->ifindex, right? >>> >>> But if skb_iff has any right to exist it should contain the first incoming >>> interface on the host IMO. >>> >>> Please correct my if i'm wrong and/or tell me what your commit message means >>> in respect to my request and why skb->dev->ifindex is not used instead of >>> skb_iif. I feel somehow lost about the skb_iif intention ... >> >> I believe that skb_iif should be removed. > > >AFAICS skb->skb_iif is used as some kind of cached variable for the original >skb->dev->ifindex - or is it really possible that skb->skb_iif is set while >skb->dev->ifindex is not accessible? > >Btw. i my case skb_iif would make sense though. > >Regards, >Oliver > >>> >>> --- >>> >>> >>> http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commitdiff;h=b68581778cd0051a3fb9a2b614dee7eccb5127ff >>> >>> net: Make skb->skb_iif always track skb->dev >>> >>> Make it follow device decapsulation, from things such as VLAN and >>> bonding. >>> >>> The stuff that actually cares about pre-demuxed device pointers, is >>> handled by the "orig_dev" variable in __netif_receive_skb(). And >>> the only consumer of that is the po->origdev feature of AF_PACKET >>> sockets. >>> >>> Signed-off-by: David S. Miller <davem@davemloft.net> >>> --- >>> >>> diff --git a/net/core/dev.c b/net/core/dev.c >>> index cca02ae..0ebaea1 100644 >>> --- a/net/core/dev.c >>> +++ b/net/core/dev.c >>> @@ -3173,8 +3173,6 @@ static int __netif_receive_skb(struct sk_buff *skb) >>> if (netpoll_receive_skb(skb)) >>> return NET_RX_DROP; >>> >>> - if (!skb->skb_iif) >>> - skb->skb_iif = skb->dev->ifindex; >>> orig_dev = skb->dev; >>> >>> skb_reset_network_header(skb); >>> @@ -3186,6 +3184,7 @@ static int __netif_receive_skb(struct sk_buff *skb) >>> rcu_read_lock(); >>> >>> another_round: >>> + skb->skb_iif = skb->dev->ifindex; >>> >>> __this_cpu_inc(softnet_data.processed); >>> >>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > >-- >To unsubscribe from this list: send the line "unsubscribe netdev" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-12 19:37 ` Jiri Pirko @ 2013-01-12 20:14 ` Oliver Hartkopp 0 siblings, 0 replies; 24+ messages in thread From: Oliver Hartkopp @ 2013-01-12 20:14 UTC (permalink / raw) To: Jiri Pirko; +Cc: David Miller, Linux Netdev List On 12.01.2013 20:37, Jiri Pirko wrote: > Sat, Jan 12, 2013 at 07:40:33PM CET, socketcan@hartkopp.net wrote: >> An there i wanted to add this code to omit sending the CAN frame on the >> originating interface: >> >> @@ .. @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data >> >> if (!(gwj->dst.dev->flags & IFF_UP)) { >> gwj->dropped_frames++; >> return; >> } >> >> + /* is sending the skb back to the incoming interface allowed? */ >> + if (!(gwj->flags & CGW_FLAGS_CAN_IIF_TX_OK) && >> + skb->skb_iif == gwj->dst.dev->ifindex) >> + return; >> + >> /* >> * clone the given skb, which has not been done in can_rcv() >> * >> >> This works fine, when the patch from Dave is reverted. >> >> I did not find any good solution to preserve the originating netdev over >> several netif_receive_skb() calls - but this skb_iif which has been made >> unusable ... > > > > Well, look at struct receiver in net/can/af_can.h: > > struct receiver { > struct hlist_node list; > struct rcu_head rcu; > canid_t can_id; > canid_t mask; > unsigned long matches; > void (*func)(struct sk_buff *, void *); > void *data; > char *ident; > }; > > your can_can_gw_rcv() is callback registered as ->func here. > This ->func is called from chain can_rcv->can_receive->can_rcv_filter->deliver > > So just extend > ->func to something like: > > void (*func)(struct sk_buff *, struct neti_device *orig_dev, void *); > and pass the orig_dev all the way through the chain. > Passing the information up to the can-gw once is not the problem. But when this skb has to be routed to another CAN netdev it is cloned and goes down from can_can_gw_rcv() to -> can_send(cloned_skb) -> dev_queue_xmit(cloned_skb) And when it has been sent successfully on the CAN bus the cloned_skb is echoed back into the system via netif_rx_ni(cloned_skb). (see http://lxr.linux.no/#linux+v3.7.2/Documentation/networking/can.txt#L177 ) This entire path - using dev_queue_xmit() / netif_rx_ni() - is reduced to a skb structure and can not deal with any orig_dev pointer. Therefore storing the first incoming interface in skb_iif is relevant for this use-case. Regards, Oliver ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-12 13:48 [RFC davem] revert: net: Make skb->skb_iif always track skb->dev Oliver Hartkopp 2013-01-12 18:13 ` Jiri Pirko @ 2013-01-12 21:23 ` David Miller 2013-01-12 21:36 ` David Miller 1 sibling, 1 reply; 24+ messages in thread From: David Miller @ 2013-01-12 21:23 UTC (permalink / raw) To: socketcan; +Cc: netdev From: Oliver Hartkopp <socketcan@hartkopp.net> Date: Sat, 12 Jan 2013 14:48:14 +0100 > To me it is not clear why skb_iff is needed anyway as the value should > always be available via skb->dev->ifindex, right? But all the code uses skb_iif, in particular the ipv4 routing lookups use that as the key. It absolutely must follow whatever is skb->dev, it is a hard invariant. I am not reverting this change. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-12 21:23 ` David Miller @ 2013-01-12 21:36 ` David Miller 2013-01-13 3:06 ` [PATCH net-next] pkt_sched: namespace aware ifb Eric Dumazet 2013-01-13 11:01 ` [RFC davem] revert: net: Make skb->skb_iif always track skb->dev Oliver Hartkopp 0 siblings, 2 replies; 24+ messages in thread From: David Miller @ 2013-01-12 21:36 UTC (permalink / raw) To: socketcan; +Cc: netdev From: David Miller <davem@davemloft.net> Date: Sat, 12 Jan 2013 13:23:16 -0800 (PST) > From: Oliver Hartkopp <socketcan@hartkopp.net> > Date: Sat, 12 Jan 2013 14:48:14 +0100 > >> To me it is not clear why skb_iff is needed anyway as the value should >> always be available via skb->dev->ifindex, right? > > But all the code uses skb_iif, in particular the ipv4 routing > lookups use that as the key. > > It absolutely must follow whatever is skb->dev, it is a hard > invariant. > > I am not reverting this change. More information, because I can't believe how idiotic and ignorant people are being able this issue. skb->dev->ifindex IS NOT the same as skb->skb_iif Why don't you put a test into tcp_recvmsg() for packets being removed from the socket's receive queue and see if skb->dev->ifindex is the same as skb->skb_iif Surprise, skb->dev is going to be NULL at that point. Why? Because on packet receive we don't take references on devices we hook into skb->dev, therefore we cannot let that pointer escape the software interrupt packet input paths. Therefore, as a bug trap, TCP input will set skb->dev to NULL. The only valid way to figure out the final demuxed device the packet arrived on, is therefore, via skb->skb_iif. As per your problem with CAN, that's also rediculous. You have an SKB control block in skb->cb[] that you can put whatever values with whatever semantics you want. Use it. I'm not discussing this any further. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-12 21:36 ` David Miller @ 2013-01-13 3:06 ` Eric Dumazet 2013-01-13 3:50 ` Benjamin LaHaise 2013-01-13 11:01 ` [RFC davem] revert: net: Make skb->skb_iif always track skb->dev Oliver Hartkopp 1 sibling, 1 reply; 24+ messages in thread From: Eric Dumazet @ 2013-01-13 3:06 UTC (permalink / raw) To: David Miller; +Cc: socketcan, netdev From: Eric Dumazet <edumazet@google.com> act_mirred needs to find the current net_ns, and struct net pointer is not provided in the call chain. We run in process context and current->nsproxy->net_ns is the needed pointer. For ifb, things are easier, as the current ifb device can provide the net pointer immediately. Signed-off-by: Eric Dumazet <edumazet@google.com> --- drivers/net/ifb.c | 2 +- net/sched/act_mirred.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c index 344dceb..8216438 100644 --- a/drivers/net/ifb.c +++ b/drivers/net/ifb.c @@ -90,7 +90,7 @@ static void ri_tasklet(unsigned long dev) u64_stats_update_end(&dp->tsync); rcu_read_lock(); - skb->dev = dev_get_by_index_rcu(&init_net, skb->skb_iif); + skb->dev = dev_get_by_index_rcu(dev_net(_dev), skb->skb_iif); if (!skb->dev) { rcu_read_unlock(); dev_kfree_skb(skb); diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 9c0fd0c..f5a7e18 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -88,7 +88,7 @@ static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est, return -EINVAL; } if (parm->ifindex) { - dev = __dev_get_by_index(&init_net, parm->ifindex); + dev = __dev_get_by_index(current->nsproxy->net_ns, parm->ifindex); if (dev == NULL) return -ENODEV; switch (dev->type) { ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 3:06 ` [PATCH net-next] pkt_sched: namespace aware ifb Eric Dumazet @ 2013-01-13 3:50 ` Benjamin LaHaise 2013-01-13 5:49 ` Eric Dumazet 0 siblings, 1 reply; 24+ messages in thread From: Benjamin LaHaise @ 2013-01-13 3:50 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Miller, socketcan, netdev On Sat, Jan 12, 2013 at 07:06:14PM -0800, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > act_mirred needs to find the current net_ns, and struct net > pointer is not provided in the call chain. We run in process > context and current->nsproxy->net_ns is the needed pointer. ... I don't think this is correct. Going by the call chain, tcf_action_add can be called because of a netlink message, and that netlink message may not be in the same "struct net" as the current process. It looks like the ->init operation is going to need to have the namespace passed in for this to work correctly. -ben > diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c > index 9c0fd0c..f5a7e18 100644 > --- a/net/sched/act_mirred.c > +++ b/net/sched/act_mirred.c > @@ -88,7 +88,7 @@ static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est, > return -EINVAL; > } > if (parm->ifindex) { > - dev = __dev_get_by_index(&init_net, parm->ifindex); > + dev = __dev_get_by_index(current->nsproxy->net_ns, parm->ifindex); > if (dev == NULL) > return -ENODEV; > switch (dev->type) { > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- "Thought is the essence of where you are now." ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 3:50 ` Benjamin LaHaise @ 2013-01-13 5:49 ` Eric Dumazet 2013-01-13 14:44 ` Jamal Hadi Salim 2013-01-13 14:59 ` [PATCH net-next] pkt_sched: namespace aware ifb Benjamin LaHaise 0 siblings, 2 replies; 24+ messages in thread From: Eric Dumazet @ 2013-01-13 5:49 UTC (permalink / raw) To: Benjamin LaHaise; +Cc: David Miller, socketcan, netdev On Sat, 2013-01-12 at 22:50 -0500, Benjamin LaHaise wrote: > On Sat, Jan 12, 2013 at 07:06:14PM -0800, Eric Dumazet wrote: > > From: Eric Dumazet <edumazet@google.com> > > > > act_mirred needs to find the current net_ns, and struct net > > pointer is not provided in the call chain. We run in process > > context and current->nsproxy->net_ns is the needed pointer. > ... > > I don't think this is correct. Going by the call chain, tcf_action_add can > be called because of a netlink message, and that netlink message may not be > in the same "struct net" as the current process. It looks like the ->init > operation is going to need to have the namespace passed in for this to work > correctly. But it is working in my tests. I added a WARN and the call stack is : [ 701.522282] [<ffffffff8108634f>] warn_slowpath_common+0x7f/0xc0 [ 701.522284] [<ffffffff810863aa>] warn_slowpath_null+0x1a/0x20 [ 701.522286] [<ffffffffa00e00e3>] tcf_mirred_init+0x43/0x340 [act_mirred] [ 701.522289] [<ffffffff81506385>] ? __rtnl_unlock+0x15/0x20 [ 701.522293] [<ffffffff815195e8>] tcf_action_init_1+0x198/0x1e0 [ 701.522295] [<ffffffff815196c8>] tcf_action_init+0x98/0x100 [ 701.522298] [<ffffffff81517f30>] tcf_exts_validate+0x90/0xb0 [ 701.522300] [<ffffffff8151e55b>] u32_set_parms.isra.11+0x3b/0x270 [ 701.522303] [<ffffffff812ffdf0>] ? nla_parse+0x90/0xe0 [ 701.522304] [<ffffffff8151ed16>] u32_change+0x2e6/0x4c0 [ 701.522306] [<ffffffff81518432>] tc_ctl_tfilter+0x4e2/0x720 [ 701.522308] [<ffffffff815064ad>] rtnetlink_rcv_msg+0x11d/0x310 [ 701.522310] [<ffffffff81506390>] ? __rtnl_unlock+0x20/0x20 [ 701.522312] [<ffffffff81522d39>] netlink_rcv_skb+0xa9/0xd0 [ 701.522314] [<ffffffff81503875>] rtnetlink_rcv+0x25/0x40 [ 701.522316] [<ffffffff81522681>] netlink_unicast+0x1b1/0x230 [ 701.522317] [<ffffffff815229fe>] netlink_sendmsg+0x2fe/0x3b0 [ 701.522321] [<ffffffff814dbdf2>] sock_sendmsg+0xd2/0xf0 [ 701.522323] [<ffffffff814dbca0>] ? sock_recvmsg+0xe0/0x100 [ 701.522326] [<ffffffff814dd0f0>] __sys_sendmsg+0x380/0x390 [ 701.522329] [<ffffffff815b20b4>] ? __do_page_fault+0x214/0x460 [ 701.522331] [<ffffffff814df4e9>] sys_sendmsg+0x49/0x90 [ 701.522334] [<ffffffff815b68c2>] system_call_fastpath+0x16/0x1b Could you elaborate on what could be the problem ? We hold the RTNL, so I dont think another process could possibly call tcf_mirred_init() ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 5:49 ` Eric Dumazet @ 2013-01-13 14:44 ` Jamal Hadi Salim 2013-01-13 16:41 ` Benjamin LaHaise 2013-01-13 17:46 ` [PATCH net-next] ifb: dont hard code inet_net use Eric Dumazet 2013-01-13 14:59 ` [PATCH net-next] pkt_sched: namespace aware ifb Benjamin LaHaise 1 sibling, 2 replies; 24+ messages in thread From: Jamal Hadi Salim @ 2013-01-13 14:44 UTC (permalink / raw) To: Eric Dumazet; +Cc: Benjamin LaHaise, David Miller, socketcan, netdev On 13-01-13 12:49 AM, Eric Dumazet wrote: > Could you elaborate on what could be the problem ? > > We hold the RTNL, so I dont think another process could possibly call > tcf_mirred_init() > Eric, the point probably Ben was trying to make is not about synchronizing rather about which namespace has the right to that action config. Your change is correct for the common use of actions but does not fix the larger picture. At the moment a dev is owned by a specific namespace; that owns a tc filter that in turn owns an action. So no problem with the change you make if all configuration follows those rules i.e something along the lines of: === tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \ match ip dst 10.0.0.229/32 flowid 1:10 \ action mirred egress redirect dev ifb0 ===== I would say most people use the above syntax. However, there is another way to configure actions so they can be shared[1], example control syntax: ---- tc actions add \ action police rate 1kbit burst 90k drop index 3 \ action mirred egress mirror dev eth0 index 5 ---- In such a case, the "tc actions" netlink path may be entered from a different namespace than the one that is using it. Then current->nsproxy->net_ns is no longer correct. To correct this, i think what Ben points out in passing the init() the correct namespace seem like the way to go. Feel free to make that change - otherwise i will get to it and fix it. cheers, jamal [1] You can then have multiple filters use this action like so: === tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \ match ip dst 10.0.0.229/32 flowid 1:10 \ action police index 3 # tc filter add dev eth0 parent ffff: protocol ip prio 6 u32 \ match ip src 10.0.0.21/32 flowid 1:16 \ action police index 3 action mirred egress mirror dev eth0 ===== ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 14:44 ` Jamal Hadi Salim @ 2013-01-13 16:41 ` Benjamin LaHaise 2013-01-13 16:57 ` Eric Dumazet 2013-01-13 17:46 ` [PATCH net-next] ifb: dont hard code inet_net use Eric Dumazet 1 sibling, 1 reply; 24+ messages in thread From: Benjamin LaHaise @ 2013-01-13 16:41 UTC (permalink / raw) To: Eric Dumazet, Jamal Hadi Salim; +Cc: David Miller, socketcan, netdev Hi folks, On Sun, Jan 13, 2013 at 09:44:48AM -0500, Jamal Hadi Salim wrote: > Eric, the point probably Ben was trying to make is not about > synchronizing rather about which namespace has the right to that action > config. Your change is correct for the common use of actions > but does not fix the larger picture. ... > In such a case, the "tc actions" netlink path may be > entered from a different namespace than the one that is > using it. Then current->nsproxy->net_ns is no longer correct. > > To correct this, i think what Ben points out in passing the > init() the correct namespace seem like the way to go. Feel free > to make that change - otherwise i will get to it and fix it. Yep, Jamal's right on the point I'm trying to make. To deal with this, I think we need a patch along the lines of the following to pass the 'struct net *' down to where it's needed... Please note that I've only compile tested this with all the net/sched modules enabled and a allmodconfig build. It's a bit bigger, but passing the argument down through the call chain looks simpler than trying to stuff a struct net pointer into the various structures and keep that in sync with the network device's network namespace. -ben -- "Thought is the essence of where you are now." -- pkt_sched: namespace aware ifb v2 Eric Dumazet pointed out that act_mirred needs to find the current net_ns, and struct net pointer is not provided in the call chain. His original patch made use of current->nsproxy->net_ns to find the network namespace, but this fails to work correctly for userspace code that makes use of netlink sockets in different network namespaces. Instead, pass the "struct net *" down along the call chain to where it is needed. Signed-off-by: Benjamin LaHaise <bcrl@kvack.org> --- drivers/net/ifb.c | 2 +- include/net/act_api.h | 12 +++++++++--- include/net/pkt_cls.h | 7 ++++--- include/net/sch_generic.h | 2 +- net/sched/act_api.c | 18 ++++++++++-------- net/sched/act_csum.c | 2 +- net/sched/act_gact.c | 5 +++-- net/sched/act_ipt.c | 2 +- net/sched/act_mirred.c | 7 ++++--- net/sched/act_nat.c | 2 +- net/sched/act_pedit.c | 5 +++-- net/sched/act_police.c | 5 +++-- net/sched/act_simple.c | 5 +++-- net/sched/act_skbedit.c | 5 +++-- net/sched/cls_api.c | 11 ++++++----- net/sched/cls_basic.c | 13 +++++++------ net/sched/cls_cgroup.c | 5 +++-- net/sched/cls_flow.c | 4 ++-- net/sched/cls_fw.c | 10 +++++----- net/sched/cls_route.c | 15 ++++++++------- net/sched/cls_rsvp.h | 4 ++-- net/sched/cls_tcindex.c | 14 ++++++++------ net/sched/cls_u32.c | 13 +++++++------ 23 files changed, 95 insertions(+), 73 deletions(-) diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c index 344dceb..8216438 100644 --- a/drivers/net/ifb.c +++ b/drivers/net/ifb.c @@ -90,7 +90,7 @@ static void ri_tasklet(unsigned long dev) u64_stats_update_end(&dp->tsync); rcu_read_lock(); - skb->dev = dev_get_by_index_rcu(&init_net, skb->skb_iif); + skb->dev = dev_get_by_index_rcu(dev_net(_dev), skb->skb_iif); if (!skb->dev) { rcu_read_unlock(); dev_kfree_skb(skb); diff --git a/include/net/act_api.h b/include/net/act_api.h index c739531..112c25c 100644 --- a/include/net/act_api.h +++ b/include/net/act_api.h @@ -91,7 +91,9 @@ struct tc_action_ops { int (*dump)(struct sk_buff *, struct tc_action *, int, int); int (*cleanup)(struct tc_action *, int bind); int (*lookup)(struct tc_action *, u32); - int (*init)(struct nlattr *, struct nlattr *, struct tc_action *, int , int); + int (*init)(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *act, int ovr, + int bind); int (*walk)(struct sk_buff *, struct netlink_callback *, int, struct tc_action *); }; @@ -116,8 +118,12 @@ extern int tcf_register_action(struct tc_action_ops *a); extern int tcf_unregister_action(struct tc_action_ops *a); extern void tcf_action_destroy(struct tc_action *a, int bind); extern int tcf_action_exec(struct sk_buff *skb, const struct tc_action *a, struct tcf_result *res); -extern struct tc_action *tcf_action_init(struct nlattr *nla, struct nlattr *est, char *n, int ovr, int bind); -extern struct tc_action *tcf_action_init_1(struct nlattr *nla, struct nlattr *est, char *n, int ovr, int bind); +extern struct tc_action *tcf_action_init(struct net *net, struct nlattr *nla, + struct nlattr *est, char *n, int ovr, + int bind); +extern struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, + struct nlattr *est, char *n, int ovr, + int bind); extern int tcf_action_dump(struct sk_buff *skb, struct tc_action *a, int, int); extern int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int); extern int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int); diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 9fcc680..1317450 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -126,9 +126,10 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, return 0; } -extern int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, - struct nlattr *rate_tlv, struct tcf_exts *exts, - const struct tcf_ext_map *map); +extern int tcf_exts_validate(struct net *net, struct tcf_proto *tp, + struct nlattr **tb, struct nlattr *rate_tlv, + struct tcf_exts *exts, + const struct tcf_ext_map *map); extern void tcf_exts_destroy(struct tcf_proto *tp, struct tcf_exts *exts); extern void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst, struct tcf_exts *src); diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 1540f9c..2d06c2a 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -195,7 +195,7 @@ struct tcf_proto_ops { unsigned long (*get)(struct tcf_proto*, u32 handle); void (*put)(struct tcf_proto*, unsigned long); - int (*change)(struct sk_buff *, + int (*change)(struct net *net, struct sk_buff *, struct tcf_proto*, unsigned long, u32 handle, struct nlattr **, unsigned long *); diff --git a/net/sched/act_api.c b/net/sched/act_api.c index 65d240c..8579c4b 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -485,8 +485,9 @@ errout: return err; } -struct tc_action *tcf_action_init_1(struct nlattr *nla, struct nlattr *est, - char *name, int ovr, int bind) +struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, + struct nlattr *est, char *name, int ovr, + int bind) { struct tc_action *a; struct tc_action_ops *a_o; @@ -542,9 +543,9 @@ struct tc_action *tcf_action_init_1(struct nlattr *nla, struct nlattr *est, /* backward compatibility for policer */ if (name == NULL) - err = a_o->init(tb[TCA_ACT_OPTIONS], est, a, ovr, bind); + err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, a, ovr, bind); else - err = a_o->init(nla, est, a, ovr, bind); + err = a_o->init(net, nla, est, a, ovr, bind); if (err < 0) goto err_free; @@ -566,8 +567,9 @@ err_out: return ERR_PTR(err); } -struct tc_action *tcf_action_init(struct nlattr *nla, struct nlattr *est, - char *name, int ovr, int bind) +struct tc_action *tcf_action_init(struct net *net, struct nlattr *nla, + struct nlattr *est, char *name, int ovr, + int bind) { struct nlattr *tb[TCA_ACT_MAX_PRIO + 1]; struct tc_action *head = NULL, *act, *act_prev = NULL; @@ -579,7 +581,7 @@ struct tc_action *tcf_action_init(struct nlattr *nla, struct nlattr *est, return ERR_PTR(err); for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) { - act = tcf_action_init_1(tb[i], est, name, ovr, bind); + act = tcf_action_init_1(net, tb[i], est, name, ovr, bind); if (IS_ERR(act)) goto err; act->order = i; @@ -960,7 +962,7 @@ tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n, struct tc_action *a; u32 seq = n->nlmsg_seq; - act = tcf_action_init(nla, NULL, NULL, ovr, 0); + act = tcf_action_init(net, nla, NULL, NULL, ovr, 0); if (act == NULL) goto done; if (IS_ERR(act)) { diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c index 2c8ad7c..08fa1e8 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -51,7 +51,7 @@ static const struct nla_policy csum_policy[TCA_CSUM_MAX + 1] = { [TCA_CSUM_PARMS] = { .len = sizeof(struct tc_csum), }, }; -static int tcf_csum_init(struct nlattr *nla, struct nlattr *est, +static int tcf_csum_init(struct net *n, struct nlattr *nla, struct nlattr *est, struct tc_action *a, int ovr, int bind) { struct nlattr *tb[TCA_CSUM_MAX + 1]; diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index 05d60859..fd2b3cf 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -58,8 +58,9 @@ static const struct nla_policy gact_policy[TCA_GACT_MAX + 1] = { [TCA_GACT_PROB] = { .len = sizeof(struct tc_gact_p) }, }; -static int tcf_gact_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_gact_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_GACT_MAX + 1]; struct tc_gact *parm; diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c index 58fb3c7..0fb9e3f 100644 --- a/net/sched/act_ipt.c +++ b/net/sched/act_ipt.c @@ -102,7 +102,7 @@ static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = { [TCA_IPT_TARG] = { .len = sizeof(struct xt_entry_target) }, }; -static int tcf_ipt_init(struct nlattr *nla, struct nlattr *est, +static int tcf_ipt_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action *a, int ovr, int bind) { struct nlattr *tb[TCA_IPT_MAX + 1]; diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 9c0fd0c..5d676ed 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -62,8 +62,9 @@ static const struct nla_policy mirred_policy[TCA_MIRRED_MAX + 1] = { [TCA_MIRRED_PARMS] = { .len = sizeof(struct tc_mirred) }, }; -static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_mirred_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, int ovr, + int bind) { struct nlattr *tb[TCA_MIRRED_MAX + 1]; struct tc_mirred *parm; @@ -88,7 +89,7 @@ static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est, return -EINVAL; } if (parm->ifindex) { - dev = __dev_get_by_index(&init_net, parm->ifindex); + dev = __dev_get_by_index(net, parm->ifindex); if (dev == NULL) return -ENODEV; switch (dev->type) { diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c index b5d029e..876f0ef 100644 --- a/net/sched/act_nat.c +++ b/net/sched/act_nat.c @@ -44,7 +44,7 @@ static const struct nla_policy nat_policy[TCA_NAT_MAX + 1] = { [TCA_NAT_PARMS] = { .len = sizeof(struct tc_nat) }, }; -static int tcf_nat_init(struct nlattr *nla, struct nlattr *est, +static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action *a, int ovr, int bind) { struct nlattr *tb[TCA_NAT_MAX + 1]; diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 45c53ab..0c3fadd 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -38,8 +38,9 @@ static const struct nla_policy pedit_policy[TCA_PEDIT_MAX + 1] = { [TCA_PEDIT_PARMS] = { .len = sizeof(struct tc_pedit) }, }; -static int tcf_pedit_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_pedit_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_PEDIT_MAX + 1]; struct tc_pedit *parm; diff --git a/net/sched/act_police.c b/net/sched/act_police.c index a9de232..8dbd695 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -130,8 +130,9 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = { [TCA_POLICE_RESULT] = { .type = NLA_U32 }, }; -static int tcf_act_police_locate(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_act_police_locate(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { unsigned int h; int ret = 0, err; diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index 3714f60..7725eb4 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -95,8 +95,9 @@ static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = { [TCA_DEF_DATA] = { .type = NLA_STRING, .len = SIMP_MAX_DATA }, }; -static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_simp_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_DEF_MAX + 1]; struct tc_defact *parm; diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index 476e0fa..cb42211 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -67,8 +67,9 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = { [TCA_SKBEDIT_MARK] = { .len = sizeof(u32) }, }; -static int tcf_skbedit_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_skbedit_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_SKBEDIT_MAX + 1]; struct tc_skbedit *parm; diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index ff55ed6..964f5e4 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -321,7 +321,7 @@ replay: } } - err = tp->ops->change(skb, tp, cl, t->tcm_handle, tca, &fh); + err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh); if (err == 0) { if (tp_created) { spin_lock_bh(root_lock); @@ -508,7 +508,7 @@ void tcf_exts_destroy(struct tcf_proto *tp, struct tcf_exts *exts) } EXPORT_SYMBOL(tcf_exts_destroy); -int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, +int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, struct nlattr *rate_tlv, struct tcf_exts *exts, const struct tcf_ext_map *map) { @@ -519,7 +519,7 @@ int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, struct tc_action *act; if (map->police && tb[map->police]) { - act = tcf_action_init_1(tb[map->police], rate_tlv, + act = tcf_action_init_1(net, tb[map->police], rate_tlv, "police", TCA_ACT_NOREPLACE, TCA_ACT_BIND); if (IS_ERR(act)) @@ -528,8 +528,9 @@ int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, act->type = TCA_OLD_COMPAT; exts->action = act; } else if (map->action && tb[map->action]) { - act = tcf_action_init(tb[map->action], rate_tlv, NULL, - TCA_ACT_NOREPLACE, TCA_ACT_BIND); + act = tcf_action_init(net, tb[map->action], rate_tlv, + NULL, TCA_ACT_NOREPLACE, + TCA_ACT_BIND); if (IS_ERR(act)) return PTR_ERR(act); diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c index 344a11b..d76a35d 100644 --- a/net/sched/cls_basic.c +++ b/net/sched/cls_basic.c @@ -132,15 +132,16 @@ static const struct nla_policy basic_policy[TCA_BASIC_MAX + 1] = { [TCA_BASIC_EMATCHES] = { .type = NLA_NESTED }, }; -static int basic_set_parms(struct tcf_proto *tp, struct basic_filter *f, - unsigned long base, struct nlattr **tb, +static int basic_set_parms(struct net *net, struct tcf_proto *tp, + struct basic_filter *f, unsigned long base, + struct nlattr **tb, struct nlattr *est) { int err = -EINVAL; struct tcf_exts e; struct tcf_ematch_tree t; - err = tcf_exts_validate(tp, tb, est, &e, &basic_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &basic_ext_map); if (err < 0) return err; @@ -162,7 +163,7 @@ errout: return err; } -static int basic_change(struct sk_buff *in_skb, +static int basic_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) { @@ -182,7 +183,7 @@ static int basic_change(struct sk_buff *in_skb, if (f != NULL) { if (handle && f->handle != handle) return -EINVAL; - return basic_set_parms(tp, f, base, tb, tca[TCA_RATE]); + return basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE]); } err = -ENOBUFS; @@ -208,7 +209,7 @@ static int basic_change(struct sk_buff *in_skb, f->handle = head->hgenerator; } - err = basic_set_parms(tp, f, base, tb, tca[TCA_RATE]); + err = basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE]); if (err < 0) goto errout; diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c index 6db7855..3a294eb 100644 --- a/net/sched/cls_cgroup.c +++ b/net/sched/cls_cgroup.c @@ -178,7 +178,7 @@ static const struct nla_policy cgroup_policy[TCA_CGROUP_MAX + 1] = { [TCA_CGROUP_EMATCHES] = { .type = NLA_NESTED }, }; -static int cls_cgroup_change(struct sk_buff *in_skb, +static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) @@ -215,7 +215,8 @@ static int cls_cgroup_change(struct sk_buff *in_skb, if (err < 0) return err; - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &cgroup_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, + &cgroup_ext_map); if (err < 0) return err; diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c index ce82d0c..e6c1427 100644 --- a/net/sched/cls_flow.c +++ b/net/sched/cls_flow.c @@ -351,7 +351,7 @@ static const struct nla_policy flow_policy[TCA_FLOW_MAX + 1] = { [TCA_FLOW_PERTURB] = { .type = NLA_U32 }, }; -static int flow_change(struct sk_buff *in_skb, +static int flow_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) @@ -397,7 +397,7 @@ static int flow_change(struct sk_buff *in_skb, return -EOPNOTSUPP; } - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &flow_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, &flow_ext_map); if (err < 0) return err; diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index 4075a0a..1135d82 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -192,7 +192,7 @@ static const struct nla_policy fw_policy[TCA_FW_MAX + 1] = { }; static int -fw_change_attrs(struct tcf_proto *tp, struct fw_filter *f, +fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f, struct nlattr **tb, struct nlattr **tca, unsigned long base) { struct fw_head *head = (struct fw_head *)tp->root; @@ -200,7 +200,7 @@ fw_change_attrs(struct tcf_proto *tp, struct fw_filter *f, u32 mask; int err; - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &fw_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, &fw_ext_map); if (err < 0) return err; @@ -233,7 +233,7 @@ errout: return err; } -static int fw_change(struct sk_buff *in_skb, +static int fw_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, @@ -255,7 +255,7 @@ static int fw_change(struct sk_buff *in_skb, if (f != NULL) { if (f->id != handle && handle) return -EINVAL; - return fw_change_attrs(tp, f, tb, tca, base); + return fw_change_attrs(net, tp, f, tb, tca, base); } if (!handle) @@ -282,7 +282,7 @@ static int fw_change(struct sk_buff *in_skb, f->id = handle; - err = fw_change_attrs(tp, f, tb, tca, base); + err = fw_change_attrs(net, tp, f, tb, tca, base); if (err < 0) goto errout; diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index c10d57b..37da567 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -335,9 +335,10 @@ static const struct nla_policy route4_policy[TCA_ROUTE4_MAX + 1] = { [TCA_ROUTE4_IIF] = { .type = NLA_U32 }, }; -static int route4_set_parms(struct tcf_proto *tp, unsigned long base, - struct route4_filter *f, u32 handle, struct route4_head *head, - struct nlattr **tb, struct nlattr *est, int new) +static int route4_set_parms(struct net *net, struct tcf_proto *tp, + unsigned long base, struct route4_filter *f, + u32 handle, struct route4_head *head, + struct nlattr **tb, struct nlattr *est, int new) { int err; u32 id = 0, to = 0, nhandle = 0x8000; @@ -346,7 +347,7 @@ static int route4_set_parms(struct tcf_proto *tp, unsigned long base, struct route4_bucket *b; struct tcf_exts e; - err = tcf_exts_validate(tp, tb, est, &e, &route_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &route_ext_map); if (err < 0) return err; @@ -427,7 +428,7 @@ errout: return err; } -static int route4_change(struct sk_buff *in_skb, +static int route4_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, @@ -457,7 +458,7 @@ static int route4_change(struct sk_buff *in_skb, if (f->bkt) old_handle = f->handle; - err = route4_set_parms(tp, base, f, handle, head, tb, + err = route4_set_parms(net, tp, base, f, handle, head, tb, tca[TCA_RATE], 0); if (err < 0) return err; @@ -480,7 +481,7 @@ static int route4_change(struct sk_buff *in_skb, if (f == NULL) goto errout; - err = route4_set_parms(tp, base, f, handle, head, tb, + err = route4_set_parms(net, tp, base, f, handle, head, tb, tca[TCA_RATE], 1); if (err < 0) goto errout; diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h index 494bbb9..252d8b0 100644 --- a/net/sched/cls_rsvp.h +++ b/net/sched/cls_rsvp.h @@ -416,7 +416,7 @@ static const struct nla_policy rsvp_policy[TCA_RSVP_MAX + 1] = { [TCA_RSVP_PINFO] = { .len = sizeof(struct tc_rsvp_pinfo) }, }; -static int rsvp_change(struct sk_buff *in_skb, +static int rsvp_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, @@ -440,7 +440,7 @@ static int rsvp_change(struct sk_buff *in_skb, if (err < 0) return err; - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &rsvp_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, &rsvp_ext_map); if (err < 0) return err; diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c index a1293b4..b86535a 100644 --- a/net/sched/cls_tcindex.c +++ b/net/sched/cls_tcindex.c @@ -197,9 +197,10 @@ static const struct nla_policy tcindex_policy[TCA_TCINDEX_MAX + 1] = { }; static int -tcindex_set_parms(struct tcf_proto *tp, unsigned long base, u32 handle, - struct tcindex_data *p, struct tcindex_filter_result *r, - struct nlattr **tb, struct nlattr *est) +tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, + u32 handle, struct tcindex_data *p, + struct tcindex_filter_result *r, struct nlattr **tb, + struct nlattr *est) { int err, balloc = 0; struct tcindex_filter_result new_filter_result, *old_r = r; @@ -208,7 +209,7 @@ tcindex_set_parms(struct tcf_proto *tp, unsigned long base, u32 handle, struct tcindex_filter *f = NULL; /* make gcc behave */ struct tcf_exts e; - err = tcf_exts_validate(tp, tb, est, &e, &tcindex_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &tcindex_ext_map); if (err < 0) return err; @@ -332,7 +333,7 @@ errout: } static int -tcindex_change(struct sk_buff *in_skb, +tcindex_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) { @@ -353,7 +354,8 @@ tcindex_change(struct sk_buff *in_skb, if (err < 0) return err; - return tcindex_set_parms(tp, base, handle, p, r, tb, tca[TCA_RATE]); + return tcindex_set_parms(net, tp, base, handle, p, r, tb, + tca[TCA_RATE]); } diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index c7c27bc..eb07a1e 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -488,15 +488,15 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = { [TCA_U32_MARK] = { .len = sizeof(struct tc_u32_mark) }, }; -static int u32_set_parms(struct tcf_proto *tp, unsigned long base, - struct tc_u_hnode *ht, +static int u32_set_parms(struct net *net, struct tcf_proto *tp, + unsigned long base, struct tc_u_hnode *ht, struct tc_u_knode *n, struct nlattr **tb, struct nlattr *est) { int err; struct tcf_exts e; - err = tcf_exts_validate(tp, tb, est, &e, &u32_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &u32_ext_map); if (err < 0) return err; @@ -544,7 +544,7 @@ errout: return err; } -static int u32_change(struct sk_buff *in_skb, +static int u32_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) @@ -570,7 +570,8 @@ static int u32_change(struct sk_buff *in_skb, if (TC_U32_KEY(n->handle) == 0) return -EINVAL; - return u32_set_parms(tp, base, n->ht_up, n, tb, tca[TCA_RATE]); + return u32_set_parms(net, tp, base, n->ht_up, n, tb, + tca[TCA_RATE]); } if (tb[TCA_U32_DIVISOR]) { @@ -656,7 +657,7 @@ static int u32_change(struct sk_buff *in_skb, } #endif - err = u32_set_parms(tp, base, ht, n, tb, tca[TCA_RATE]); + err = u32_set_parms(net, tp, base, ht, n, tb, tca[TCA_RATE]); if (err == 0) { struct tc_u_knode **ins; for (ins = &ht->ht[TC_U32_HASH(handle)]; *ins; ins = &(*ins)->next) ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 16:41 ` Benjamin LaHaise @ 2013-01-13 16:57 ` Eric Dumazet 2013-01-13 17:25 ` Jamal Hadi Salim 2013-01-14 5:40 ` Eric Dumazet 0 siblings, 2 replies; 24+ messages in thread From: Eric Dumazet @ 2013-01-13 16:57 UTC (permalink / raw) To: Benjamin LaHaise; +Cc: Jamal Hadi Salim, David Miller, socketcan, netdev On Sun, 2013-01-13 at 11:41 -0500, Benjamin LaHaise wrote: > Hi folks, > > On Sun, Jan 13, 2013 at 09:44:48AM -0500, Jamal Hadi Salim wrote: > > Eric, the point probably Ben was trying to make is not about > > synchronizing rather about which namespace has the right to that action > > config. Your change is correct for the common use of actions > > but does not fix the larger picture. > ... > > In such a case, the "tc actions" netlink path may be > > entered from a different namespace than the one that is > > using it. Then current->nsproxy->net_ns is no longer correct. > > > > To correct this, i think what Ben points out in passing the > > init() the correct namespace seem like the way to go. Feel free > > to make that change - otherwise i will get to it and fix it. > > Yep, Jamal's right on the point I'm trying to make. To deal with this, I > think we need a patch along the lines of the following to pass the 'struct > net *' down to where it's needed... Please note that I've only compile > tested this with all the net/sched modules enabled and a allmodconfig > build. It's a bit bigger, but passing the argument down through the call > chain looks simpler than trying to stuff a struct net pointer into the > various structures and keep that in sync with the network device's network > namespace. > > -ben > -- > "Thought is the essence of where you are now." > > -- > pkt_sched: namespace aware ifb v2 > > Eric Dumazet pointed out that act_mirred needs to find the current net_ns, > and struct net pointer is not provided in the call chain. His original > patch made use of current->nsproxy->net_ns to find the network namespace, > but this fails to work correctly for userspace code that makes use of > netlink sockets in different network namespaces. Instead, pass the > "struct net *" down along the call chain to where it is needed. > > Signed-off-by: Benjamin LaHaise <bcrl@kvack.org> > --- OK I'll test it at the end of the (sunny) day. Thanks ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 16:57 ` Eric Dumazet @ 2013-01-13 17:25 ` Jamal Hadi Salim 2013-01-14 5:40 ` Eric Dumazet 1 sibling, 0 replies; 24+ messages in thread From: Jamal Hadi Salim @ 2013-01-13 17:25 UTC (permalink / raw) To: Eric Dumazet; +Cc: Benjamin LaHaise, David Miller, socketcan, netdev On 13-01-13 11:57 AM, Eric Dumazet wrote: >> Signed-off-by: Benjamin LaHaise <bcrl@kvack.org> >> --- > > OK I'll test it at the end of the (sunny) day. And from my side this looks good. I think the ifb piece is a separate patch. Please add my ack/signoff when you make the submission. [Sorry dont have time to test it today or next 2-3 days (but you can be assured i will test).] cheers, jamal ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 16:57 ` Eric Dumazet 2013-01-13 17:25 ` Jamal Hadi Salim @ 2013-01-14 5:40 ` Eric Dumazet 1 sibling, 0 replies; 24+ messages in thread From: Eric Dumazet @ 2013-01-14 5:40 UTC (permalink / raw) To: Benjamin LaHaise; +Cc: Jamal Hadi Salim, David Miller, socketcan, netdev On Sun, 2013-01-13 at 08:57 -0800, Eric Dumazet wrote: > On Sun, 2013-01-13 at 11:41 -0500, Benjamin LaHaise wrote: > > pkt_sched: namespace aware ifb v2 > > > > Eric Dumazet pointed out that act_mirred needs to find the current net_ns, > > and struct net pointer is not provided in the call chain. His original > > patch made use of current->nsproxy->net_ns to find the network namespace, > > but this fails to work correctly for userspace code that makes use of > > netlink sockets in different network namespaces. Instead, pass the > > "struct net *" down along the call chain to where it is needed. > > > > Signed-off-by: Benjamin LaHaise <bcrl@kvack.org> > > --- > > OK I'll test it at the end of the (sunny) day. > Could you resend the patch without the ifb part, I tested it and it worked well for me, thanks ! ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next] ifb: dont hard code inet_net use 2013-01-13 14:44 ` Jamal Hadi Salim 2013-01-13 16:41 ` Benjamin LaHaise @ 2013-01-13 17:46 ` Eric Dumazet 2013-01-14 13:13 ` Jamal Hadi Salim 1 sibling, 1 reply; 24+ messages in thread From: Eric Dumazet @ 2013-01-13 17:46 UTC (permalink / raw) To: Jamal Hadi Salim; +Cc: Benjamin LaHaise, David Miller, netdev From: Eric Dumazet <edumazet@google.com> ifb should lookup devices in the appropriate namespace. Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Jamal Hadi Salim <jhs@mojatatu.com> Cc: Benjamin LaHaise <bcrl@kvack.org> --- drivers/net/ifb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c index 344dceb..8216438 100644 --- a/drivers/net/ifb.c +++ b/drivers/net/ifb.c @@ -90,7 +90,7 @@ static void ri_tasklet(unsigned long dev) u64_stats_update_end(&dp->tsync); rcu_read_lock(); - skb->dev = dev_get_by_index_rcu(&init_net, skb->skb_iif); + skb->dev = dev_get_by_index_rcu(dev_net(_dev), skb->skb_iif); if (!skb->dev) { rcu_read_unlock(); dev_kfree_skb(skb); ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] ifb: dont hard code inet_net use 2013-01-13 17:46 ` [PATCH net-next] ifb: dont hard code inet_net use Eric Dumazet @ 2013-01-14 13:13 ` Jamal Hadi Salim 2013-01-14 15:15 ` [PATCH net-next] pkt_sched: namespace aware act_mirred Benjamin LaHaise 2013-01-14 20:13 ` [PATCH net-next] ifb: dont hard code inet_net use David Miller 0 siblings, 2 replies; 24+ messages in thread From: Jamal Hadi Salim @ 2013-01-14 13:13 UTC (permalink / raw) To: Eric Dumazet; +Cc: Benjamin LaHaise, David Miller, netdev On 13-01-13 12:46 PM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > ifb should lookup devices in the appropriate namespace. > > Signed-off-by: Eric Dumazet <edumazet@google.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> cheers, jamal ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next] pkt_sched: namespace aware act_mirred 2013-01-14 13:13 ` Jamal Hadi Salim @ 2013-01-14 15:15 ` Benjamin LaHaise 2013-01-14 20:10 ` David Miller 2013-01-14 20:13 ` [PATCH net-next] ifb: dont hard code inet_net use David Miller 1 sibling, 1 reply; 24+ messages in thread From: Benjamin LaHaise @ 2013-01-14 15:15 UTC (permalink / raw) To: David Miller; +Cc: Eric Dumazet, Jamal Hadi Salim, netdev Eric Dumazet pointed out that act_mirred needs to find the current net_ns, and struct net pointer is not provided in the call chain. His original patch made use of current->nsproxy->net_ns to find the network namespace, but this fails to work correctly for userspace code that makes use of netlink sockets in different network namespaces. Instead, pass the "struct net *" down along the call chain to where it is needed. This version removes the ifb changes as Eric has submitted that patch separately, but is otherwise identical to the previous version. Signed-off-by: Benjamin LaHaise <bcrl@kvack.org> Tested-by: Eric Dumazet <eric.dumazet@gmail.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> --- include/net/act_api.h | 12 +++++++++--- include/net/pkt_cls.h | 7 ++++--- include/net/sch_generic.h | 2 +- net/sched/act_api.c | 18 ++++++++++-------- net/sched/act_csum.c | 2 +- net/sched/act_gact.c | 5 +++-- net/sched/act_ipt.c | 2 +- net/sched/act_mirred.c | 7 ++++--- net/sched/act_nat.c | 2 +- net/sched/act_pedit.c | 5 +++-- net/sched/act_police.c | 5 +++-- net/sched/act_simple.c | 5 +++-- net/sched/act_skbedit.c | 5 +++-- net/sched/cls_api.c | 11 ++++++----- net/sched/cls_basic.c | 13 +++++++------ net/sched/cls_cgroup.c | 5 +++-- net/sched/cls_flow.c | 4 ++-- net/sched/cls_fw.c | 10 +++++----- net/sched/cls_route.c | 15 ++++++++------- net/sched/cls_rsvp.h | 4 ++-- net/sched/cls_tcindex.c | 14 ++++++++------ net/sched/cls_u32.c | 13 +++++++------ 22 files changed, 94 insertions(+), 72 deletions(-) diff --git a/include/net/act_api.h b/include/net/act_api.h index c739531..112c25c 100644 --- a/include/net/act_api.h +++ b/include/net/act_api.h @@ -91,7 +91,9 @@ struct tc_action_ops { int (*dump)(struct sk_buff *, struct tc_action *, int, int); int (*cleanup)(struct tc_action *, int bind); int (*lookup)(struct tc_action *, u32); - int (*init)(struct nlattr *, struct nlattr *, struct tc_action *, int , int); + int (*init)(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *act, int ovr, + int bind); int (*walk)(struct sk_buff *, struct netlink_callback *, int, struct tc_action *); }; @@ -116,8 +118,12 @@ extern int tcf_register_action(struct tc_action_ops *a); extern int tcf_unregister_action(struct tc_action_ops *a); extern void tcf_action_destroy(struct tc_action *a, int bind); extern int tcf_action_exec(struct sk_buff *skb, const struct tc_action *a, struct tcf_result *res); -extern struct tc_action *tcf_action_init(struct nlattr *nla, struct nlattr *est, char *n, int ovr, int bind); -extern struct tc_action *tcf_action_init_1(struct nlattr *nla, struct nlattr *est, char *n, int ovr, int bind); +extern struct tc_action *tcf_action_init(struct net *net, struct nlattr *nla, + struct nlattr *est, char *n, int ovr, + int bind); +extern struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, + struct nlattr *est, char *n, int ovr, + int bind); extern int tcf_action_dump(struct sk_buff *skb, struct tc_action *a, int, int); extern int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int); extern int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int); diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 9fcc680..1317450 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -126,9 +126,10 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, return 0; } -extern int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, - struct nlattr *rate_tlv, struct tcf_exts *exts, - const struct tcf_ext_map *map); +extern int tcf_exts_validate(struct net *net, struct tcf_proto *tp, + struct nlattr **tb, struct nlattr *rate_tlv, + struct tcf_exts *exts, + const struct tcf_ext_map *map); extern void tcf_exts_destroy(struct tcf_proto *tp, struct tcf_exts *exts); extern void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst, struct tcf_exts *src); diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 1540f9c..2d06c2a 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -195,7 +195,7 @@ struct tcf_proto_ops { unsigned long (*get)(struct tcf_proto*, u32 handle); void (*put)(struct tcf_proto*, unsigned long); - int (*change)(struct sk_buff *, + int (*change)(struct net *net, struct sk_buff *, struct tcf_proto*, unsigned long, u32 handle, struct nlattr **, unsigned long *); diff --git a/net/sched/act_api.c b/net/sched/act_api.c index 65d240c..8579c4b 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -485,8 +485,9 @@ errout: return err; } -struct tc_action *tcf_action_init_1(struct nlattr *nla, struct nlattr *est, - char *name, int ovr, int bind) +struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, + struct nlattr *est, char *name, int ovr, + int bind) { struct tc_action *a; struct tc_action_ops *a_o; @@ -542,9 +543,9 @@ struct tc_action *tcf_action_init_1(struct nlattr *nla, struct nlattr *est, /* backward compatibility for policer */ if (name == NULL) - err = a_o->init(tb[TCA_ACT_OPTIONS], est, a, ovr, bind); + err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, a, ovr, bind); else - err = a_o->init(nla, est, a, ovr, bind); + err = a_o->init(net, nla, est, a, ovr, bind); if (err < 0) goto err_free; @@ -566,8 +567,9 @@ err_out: return ERR_PTR(err); } -struct tc_action *tcf_action_init(struct nlattr *nla, struct nlattr *est, - char *name, int ovr, int bind) +struct tc_action *tcf_action_init(struct net *net, struct nlattr *nla, + struct nlattr *est, char *name, int ovr, + int bind) { struct nlattr *tb[TCA_ACT_MAX_PRIO + 1]; struct tc_action *head = NULL, *act, *act_prev = NULL; @@ -579,7 +581,7 @@ struct tc_action *tcf_action_init(struct nlattr *nla, struct nlattr *est, return ERR_PTR(err); for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) { - act = tcf_action_init_1(tb[i], est, name, ovr, bind); + act = tcf_action_init_1(net, tb[i], est, name, ovr, bind); if (IS_ERR(act)) goto err; act->order = i; @@ -960,7 +962,7 @@ tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n, struct tc_action *a; u32 seq = n->nlmsg_seq; - act = tcf_action_init(nla, NULL, NULL, ovr, 0); + act = tcf_action_init(net, nla, NULL, NULL, ovr, 0); if (act == NULL) goto done; if (IS_ERR(act)) { diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c index 2c8ad7c..08fa1e8 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -51,7 +51,7 @@ static const struct nla_policy csum_policy[TCA_CSUM_MAX + 1] = { [TCA_CSUM_PARMS] = { .len = sizeof(struct tc_csum), }, }; -static int tcf_csum_init(struct nlattr *nla, struct nlattr *est, +static int tcf_csum_init(struct net *n, struct nlattr *nla, struct nlattr *est, struct tc_action *a, int ovr, int bind) { struct nlattr *tb[TCA_CSUM_MAX + 1]; diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index 05d60859..fd2b3cf 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -58,8 +58,9 @@ static const struct nla_policy gact_policy[TCA_GACT_MAX + 1] = { [TCA_GACT_PROB] = { .len = sizeof(struct tc_gact_p) }, }; -static int tcf_gact_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_gact_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_GACT_MAX + 1]; struct tc_gact *parm; diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c index 58fb3c7..0fb9e3f 100644 --- a/net/sched/act_ipt.c +++ b/net/sched/act_ipt.c @@ -102,7 +102,7 @@ static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = { [TCA_IPT_TARG] = { .len = sizeof(struct xt_entry_target) }, }; -static int tcf_ipt_init(struct nlattr *nla, struct nlattr *est, +static int tcf_ipt_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action *a, int ovr, int bind) { struct nlattr *tb[TCA_IPT_MAX + 1]; diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 9c0fd0c..5d676ed 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -62,8 +62,9 @@ static const struct nla_policy mirred_policy[TCA_MIRRED_MAX + 1] = { [TCA_MIRRED_PARMS] = { .len = sizeof(struct tc_mirred) }, }; -static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_mirred_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, int ovr, + int bind) { struct nlattr *tb[TCA_MIRRED_MAX + 1]; struct tc_mirred *parm; @@ -88,7 +89,7 @@ static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est, return -EINVAL; } if (parm->ifindex) { - dev = __dev_get_by_index(&init_net, parm->ifindex); + dev = __dev_get_by_index(net, parm->ifindex); if (dev == NULL) return -ENODEV; switch (dev->type) { diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c index b5d029e..876f0ef 100644 --- a/net/sched/act_nat.c +++ b/net/sched/act_nat.c @@ -44,7 +44,7 @@ static const struct nla_policy nat_policy[TCA_NAT_MAX + 1] = { [TCA_NAT_PARMS] = { .len = sizeof(struct tc_nat) }, }; -static int tcf_nat_init(struct nlattr *nla, struct nlattr *est, +static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action *a, int ovr, int bind) { struct nlattr *tb[TCA_NAT_MAX + 1]; diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 45c53ab..0c3fadd 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -38,8 +38,9 @@ static const struct nla_policy pedit_policy[TCA_PEDIT_MAX + 1] = { [TCA_PEDIT_PARMS] = { .len = sizeof(struct tc_pedit) }, }; -static int tcf_pedit_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_pedit_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_PEDIT_MAX + 1]; struct tc_pedit *parm; diff --git a/net/sched/act_police.c b/net/sched/act_police.c index a9de232..8dbd695 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -130,8 +130,9 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = { [TCA_POLICE_RESULT] = { .type = NLA_U32 }, }; -static int tcf_act_police_locate(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_act_police_locate(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { unsigned int h; int ret = 0, err; diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index 3714f60..7725eb4 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -95,8 +95,9 @@ static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = { [TCA_DEF_DATA] = { .type = NLA_STRING, .len = SIMP_MAX_DATA }, }; -static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_simp_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_DEF_MAX + 1]; struct tc_defact *parm; diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index 476e0fa..cb42211 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -67,8 +67,9 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = { [TCA_SKBEDIT_MARK] = { .len = sizeof(u32) }, }; -static int tcf_skbedit_init(struct nlattr *nla, struct nlattr *est, - struct tc_action *a, int ovr, int bind) +static int tcf_skbedit_init(struct net *net, struct nlattr *nla, + struct nlattr *est, struct tc_action *a, + int ovr, int bind) { struct nlattr *tb[TCA_SKBEDIT_MAX + 1]; struct tc_skbedit *parm; diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index ff55ed6..964f5e4 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -321,7 +321,7 @@ replay: } } - err = tp->ops->change(skb, tp, cl, t->tcm_handle, tca, &fh); + err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh); if (err == 0) { if (tp_created) { spin_lock_bh(root_lock); @@ -508,7 +508,7 @@ void tcf_exts_destroy(struct tcf_proto *tp, struct tcf_exts *exts) } EXPORT_SYMBOL(tcf_exts_destroy); -int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, +int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, struct nlattr *rate_tlv, struct tcf_exts *exts, const struct tcf_ext_map *map) { @@ -519,7 +519,7 @@ int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, struct tc_action *act; if (map->police && tb[map->police]) { - act = tcf_action_init_1(tb[map->police], rate_tlv, + act = tcf_action_init_1(net, tb[map->police], rate_tlv, "police", TCA_ACT_NOREPLACE, TCA_ACT_BIND); if (IS_ERR(act)) @@ -528,8 +528,9 @@ int tcf_exts_validate(struct tcf_proto *tp, struct nlattr **tb, act->type = TCA_OLD_COMPAT; exts->action = act; } else if (map->action && tb[map->action]) { - act = tcf_action_init(tb[map->action], rate_tlv, NULL, - TCA_ACT_NOREPLACE, TCA_ACT_BIND); + act = tcf_action_init(net, tb[map->action], rate_tlv, + NULL, TCA_ACT_NOREPLACE, + TCA_ACT_BIND); if (IS_ERR(act)) return PTR_ERR(act); diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c index 344a11b..d76a35d 100644 --- a/net/sched/cls_basic.c +++ b/net/sched/cls_basic.c @@ -132,15 +132,16 @@ static const struct nla_policy basic_policy[TCA_BASIC_MAX + 1] = { [TCA_BASIC_EMATCHES] = { .type = NLA_NESTED }, }; -static int basic_set_parms(struct tcf_proto *tp, struct basic_filter *f, - unsigned long base, struct nlattr **tb, +static int basic_set_parms(struct net *net, struct tcf_proto *tp, + struct basic_filter *f, unsigned long base, + struct nlattr **tb, struct nlattr *est) { int err = -EINVAL; struct tcf_exts e; struct tcf_ematch_tree t; - err = tcf_exts_validate(tp, tb, est, &e, &basic_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &basic_ext_map); if (err < 0) return err; @@ -162,7 +163,7 @@ errout: return err; } -static int basic_change(struct sk_buff *in_skb, +static int basic_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) { @@ -182,7 +183,7 @@ static int basic_change(struct sk_buff *in_skb, if (f != NULL) { if (handle && f->handle != handle) return -EINVAL; - return basic_set_parms(tp, f, base, tb, tca[TCA_RATE]); + return basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE]); } err = -ENOBUFS; @@ -208,7 +209,7 @@ static int basic_change(struct sk_buff *in_skb, f->handle = head->hgenerator; } - err = basic_set_parms(tp, f, base, tb, tca[TCA_RATE]); + err = basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE]); if (err < 0) goto errout; diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c index 6db7855..3a294eb 100644 --- a/net/sched/cls_cgroup.c +++ b/net/sched/cls_cgroup.c @@ -178,7 +178,7 @@ static const struct nla_policy cgroup_policy[TCA_CGROUP_MAX + 1] = { [TCA_CGROUP_EMATCHES] = { .type = NLA_NESTED }, }; -static int cls_cgroup_change(struct sk_buff *in_skb, +static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) @@ -215,7 +215,8 @@ static int cls_cgroup_change(struct sk_buff *in_skb, if (err < 0) return err; - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &cgroup_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, + &cgroup_ext_map); if (err < 0) return err; diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c index ce82d0c..aa36a8c 100644 --- a/net/sched/cls_flow.c +++ b/net/sched/cls_flow.c @@ -351,7 +351,7 @@ static const struct nla_policy flow_policy[TCA_FLOW_MAX + 1] = { [TCA_FLOW_PERTURB] = { .type = NLA_U32 }, }; -static int flow_change(struct sk_buff *in_skb, +static int flow_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) @@ -397,7 +397,7 @@ static int flow_change(struct sk_buff *in_skb, return -EOPNOTSUPP; } - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &flow_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, &flow_ext_map); if (err < 0) return err; diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index 4075a0a..1135d82 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -192,7 +192,7 @@ static const struct nla_policy fw_policy[TCA_FW_MAX + 1] = { }; static int -fw_change_attrs(struct tcf_proto *tp, struct fw_filter *f, +fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f, struct nlattr **tb, struct nlattr **tca, unsigned long base) { struct fw_head *head = (struct fw_head *)tp->root; @@ -200,7 +200,7 @@ fw_change_attrs(struct tcf_proto *tp, struct fw_filter *f, u32 mask; int err; - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &fw_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, &fw_ext_map); if (err < 0) return err; @@ -233,7 +233,7 @@ errout: return err; } -static int fw_change(struct sk_buff *in_skb, +static int fw_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, @@ -255,7 +255,7 @@ static int fw_change(struct sk_buff *in_skb, if (f != NULL) { if (f->id != handle && handle) return -EINVAL; - return fw_change_attrs(tp, f, tb, tca, base); + return fw_change_attrs(net, tp, f, tb, tca, base); } if (!handle) @@ -282,7 +282,7 @@ static int fw_change(struct sk_buff *in_skb, f->id = handle; - err = fw_change_attrs(tp, f, tb, tca, base); + err = fw_change_attrs(net, tp, f, tb, tca, base); if (err < 0) goto errout; diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index c10d57b..37da567 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -335,9 +335,10 @@ static const struct nla_policy route4_policy[TCA_ROUTE4_MAX + 1] = { [TCA_ROUTE4_IIF] = { .type = NLA_U32 }, }; -static int route4_set_parms(struct tcf_proto *tp, unsigned long base, - struct route4_filter *f, u32 handle, struct route4_head *head, - struct nlattr **tb, struct nlattr *est, int new) +static int route4_set_parms(struct net *net, struct tcf_proto *tp, + unsigned long base, struct route4_filter *f, + u32 handle, struct route4_head *head, + struct nlattr **tb, struct nlattr *est, int new) { int err; u32 id = 0, to = 0, nhandle = 0x8000; @@ -346,7 +347,7 @@ static int route4_set_parms(struct tcf_proto *tp, unsigned long base, struct route4_bucket *b; struct tcf_exts e; - err = tcf_exts_validate(tp, tb, est, &e, &route_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &route_ext_map); if (err < 0) return err; @@ -427,7 +428,7 @@ errout: return err; } -static int route4_change(struct sk_buff *in_skb, +static int route4_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, @@ -457,7 +458,7 @@ static int route4_change(struct sk_buff *in_skb, if (f->bkt) old_handle = f->handle; - err = route4_set_parms(tp, base, f, handle, head, tb, + err = route4_set_parms(net, tp, base, f, handle, head, tb, tca[TCA_RATE], 0); if (err < 0) return err; @@ -480,7 +481,7 @@ static int route4_change(struct sk_buff *in_skb, if (f == NULL) goto errout; - err = route4_set_parms(tp, base, f, handle, head, tb, + err = route4_set_parms(net, tp, base, f, handle, head, tb, tca[TCA_RATE], 1); if (err < 0) goto errout; diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h index 494bbb9..252d8b0 100644 --- a/net/sched/cls_rsvp.h +++ b/net/sched/cls_rsvp.h @@ -416,7 +416,7 @@ static const struct nla_policy rsvp_policy[TCA_RSVP_MAX + 1] = { [TCA_RSVP_PINFO] = { .len = sizeof(struct tc_rsvp_pinfo) }, }; -static int rsvp_change(struct sk_buff *in_skb, +static int rsvp_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, @@ -440,7 +440,7 @@ static int rsvp_change(struct sk_buff *in_skb, if (err < 0) return err; - err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &rsvp_ext_map); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, &rsvp_ext_map); if (err < 0) return err; diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c index a1293b4..b86535a 100644 --- a/net/sched/cls_tcindex.c +++ b/net/sched/cls_tcindex.c @@ -197,9 +197,10 @@ static const struct nla_policy tcindex_policy[TCA_TCINDEX_MAX + 1] = { }; static int -tcindex_set_parms(struct tcf_proto *tp, unsigned long base, u32 handle, - struct tcindex_data *p, struct tcindex_filter_result *r, - struct nlattr **tb, struct nlattr *est) +tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, + u32 handle, struct tcindex_data *p, + struct tcindex_filter_result *r, struct nlattr **tb, + struct nlattr *est) { int err, balloc = 0; struct tcindex_filter_result new_filter_result, *old_r = r; @@ -208,7 +209,7 @@ tcindex_set_parms(struct tcf_proto *tp, unsigned long base, u32 handle, struct tcindex_filter *f = NULL; /* make gcc behave */ struct tcf_exts e; - err = tcf_exts_validate(tp, tb, est, &e, &tcindex_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &tcindex_ext_map); if (err < 0) return err; @@ -332,7 +333,7 @@ errout: } static int -tcindex_change(struct sk_buff *in_skb, +tcindex_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) { @@ -353,7 +354,8 @@ tcindex_change(struct sk_buff *in_skb, if (err < 0) return err; - return tcindex_set_parms(tp, base, handle, p, r, tb, tca[TCA_RATE]); + return tcindex_set_parms(net, tp, base, handle, p, r, tb, + tca[TCA_RATE]); } diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index c7c27bc..eb07a1e 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -488,15 +488,15 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = { [TCA_U32_MARK] = { .len = sizeof(struct tc_u32_mark) }, }; -static int u32_set_parms(struct tcf_proto *tp, unsigned long base, - struct tc_u_hnode *ht, +static int u32_set_parms(struct net *net, struct tcf_proto *tp, + unsigned long base, struct tc_u_hnode *ht, struct tc_u_knode *n, struct nlattr **tb, struct nlattr *est) { int err; struct tcf_exts e; - err = tcf_exts_validate(tp, tb, est, &e, &u32_ext_map); + err = tcf_exts_validate(net, tp, tb, est, &e, &u32_ext_map); if (err < 0) return err; @@ -544,7 +544,7 @@ errout: return err; } -static int u32_change(struct sk_buff *in_skb, +static int u32_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, unsigned long *arg) @@ -570,7 +570,8 @@ static int u32_change(struct sk_buff *in_skb, if (TC_U32_KEY(n->handle) == 0) return -EINVAL; - return u32_set_parms(tp, base, n->ht_up, n, tb, tca[TCA_RATE]); + return u32_set_parms(net, tp, base, n->ht_up, n, tb, + tca[TCA_RATE]); } if (tb[TCA_U32_DIVISOR]) { @@ -656,7 +657,7 @@ static int u32_change(struct sk_buff *in_skb, } #endif - err = u32_set_parms(tp, base, ht, n, tb, tca[TCA_RATE]); + err = u32_set_parms(net, tp, base, ht, n, tb, tca[TCA_RATE]); if (err == 0) { struct tc_u_knode **ins; for (ins = &ht->ht[TC_U32_HASH(handle)]; *ins; ins = &(*ins)->next) ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware act_mirred 2013-01-14 15:15 ` [PATCH net-next] pkt_sched: namespace aware act_mirred Benjamin LaHaise @ 2013-01-14 20:10 ` David Miller 0 siblings, 0 replies; 24+ messages in thread From: David Miller @ 2013-01-14 20:10 UTC (permalink / raw) To: bcrl; +Cc: eric.dumazet, jhs, netdev From: Benjamin LaHaise <bcrl@kvack.org> Date: Mon, 14 Jan 2013 10:15:39 -0500 > Eric Dumazet pointed out that act_mirred needs to find the current net_ns, > and struct net pointer is not provided in the call chain. His original > patch made use of current->nsproxy->net_ns to find the network namespace, > but this fails to work correctly for userspace code that makes use of > netlink sockets in different network namespaces. Instead, pass the > "struct net *" down along the call chain to where it is needed. > > This version removes the ifb changes as Eric has submitted that patch > separately, but is otherwise identical to the previous version. > > Signed-off-by: Benjamin LaHaise <bcrl@kvack.org> > Tested-by: Eric Dumazet <eric.dumazet@gmail.com> > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Applied, thanks. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] ifb: dont hard code inet_net use 2013-01-14 13:13 ` Jamal Hadi Salim 2013-01-14 15:15 ` [PATCH net-next] pkt_sched: namespace aware act_mirred Benjamin LaHaise @ 2013-01-14 20:13 ` David Miller 1 sibling, 0 replies; 24+ messages in thread From: David Miller @ 2013-01-14 20:13 UTC (permalink / raw) To: jhs; +Cc: eric.dumazet, bcrl, netdev From: Jamal Hadi Salim <jhs@mojatatu.com> Date: Mon, 14 Jan 2013 08:13:45 -0500 > On 13-01-13 12:46 PM, Eric Dumazet wrote: >> From: Eric Dumazet <edumazet@google.com> >> >> ifb should lookup devices in the appropriate namespace. >> >> Signed-off-by: Eric Dumazet <edumazet@google.com> > > > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Jamal, please don't put leading whitespace in your signoffs and acks, the automated tools that look for these tags in email threads looks for them to start in the first column. I've applied this patch, thanks. Thanks. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 5:49 ` Eric Dumazet 2013-01-13 14:44 ` Jamal Hadi Salim @ 2013-01-13 14:59 ` Benjamin LaHaise 2013-01-13 16:35 ` Eric Dumazet 1 sibling, 1 reply; 24+ messages in thread From: Benjamin LaHaise @ 2013-01-13 14:59 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Miller, socketcan, netdev On Sat, Jan 12, 2013 at 09:49:59PM -0800, Eric Dumazet wrote: > But it is working in my tests. > > I added a WARN and the call stack is : ... > Could you elaborate on what could be the problem ? > > We hold the RTNL, so I dont think another process could possibly call > tcf_mirred_init() The locking isn't the issue, but how the network namespace is selected it. I've implemented some virtual router functionality using network namespaces, and prior to having the setns() syscall, the only way to manipulate other network namespaces was via socket passing between threads in different namespaces. One of the optimizations in using this technique was to open a netlink socket in another namespace, then pass that file descriptor back to the main daemon. The code could then add routes and manipulate other areas of the network stack via that netlink socket. I think this technique is a valid approach for making use of network namespaces. It also has the benefit of avoid the use of setns() for the vast majority of operations. -ben -- "Thought is the essence of where you are now." ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next] pkt_sched: namespace aware ifb 2013-01-13 14:59 ` [PATCH net-next] pkt_sched: namespace aware ifb Benjamin LaHaise @ 2013-01-13 16:35 ` Eric Dumazet 0 siblings, 0 replies; 24+ messages in thread From: Eric Dumazet @ 2013-01-13 16:35 UTC (permalink / raw) To: Benjamin LaHaise; +Cc: David Miller, socketcan, netdev On Sun, 2013-01-13 at 09:59 -0500, Benjamin LaHaise wrote: > > The locking isn't the issue, but how the network namespace is selected it. > I've implemented some virtual router functionality using network namespaces, > and prior to having the setns() syscall, the only way to manipulate other > network namespaces was via socket passing between threads in different > namespaces. One of the optimizations in using this technique was to open a > netlink socket in another namespace, then pass that file descriptor back to > the main daemon. The code could then add routes and manipulate other areas > of the network stack via that netlink socket. > OK thats evil, I'll pass the net pointer then. Thanks ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-12 21:36 ` David Miller 2013-01-13 3:06 ` [PATCH net-next] pkt_sched: namespace aware ifb Eric Dumazet @ 2013-01-13 11:01 ` Oliver Hartkopp 2013-01-13 13:20 ` David Miller 1 sibling, 1 reply; 24+ messages in thread From: Oliver Hartkopp @ 2013-01-13 11:01 UTC (permalink / raw) To: David Miller; +Cc: netdev On 12.01.2013 22:36, David Miller wrote: > As per your problem with CAN, that's also rediculous. You have an SKB > control block in skb->cb[] that you can put whatever values with > whatever semantics you want. > > Use it. I'm not writing a RFC to you, when i'm not sure having checked several options before. In the tx path already net/sched is using cb[] for it's purposes. Adding the information somewhere at the end of cb[] to not interfere with sched becomes tricky. See users of qdisc_cb_private_validate(). So the 'incoming interface index' could be stored safely in the tx path in - skb->data - skb_shared_info - skb_iif skb_iif was the obvious choice then. And the question why a seven year old if-statement in net/core/dev.c has been removed must be allowed. Networking is not only IP networking. Oliver ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC davem] revert: net: Make skb->skb_iif always track skb->dev 2013-01-13 11:01 ` [RFC davem] revert: net: Make skb->skb_iif always track skb->dev Oliver Hartkopp @ 2013-01-13 13:20 ` David Miller 0 siblings, 0 replies; 24+ messages in thread From: David Miller @ 2013-01-13 13:20 UTC (permalink / raw) To: socketcan; +Cc: netdev From: Oliver Hartkopp <socketcan@hartkopp.net> Date: Sun, 13 Jan 2013 12:01:53 +0100 > Networking is not only IP networking. But it is who the majority of the infrastructure is goint to be optimized for, and this is never going to change. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2013-01-14 20:13 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-12 13:48 [RFC davem] revert: net: Make skb->skb_iif always track skb->dev Oliver Hartkopp 2013-01-12 18:13 ` Jiri Pirko 2013-01-12 18:40 ` Oliver Hartkopp 2013-01-12 19:37 ` Jiri Pirko 2013-01-12 20:14 ` Oliver Hartkopp 2013-01-12 21:23 ` David Miller 2013-01-12 21:36 ` David Miller 2013-01-13 3:06 ` [PATCH net-next] pkt_sched: namespace aware ifb Eric Dumazet 2013-01-13 3:50 ` Benjamin LaHaise 2013-01-13 5:49 ` Eric Dumazet 2013-01-13 14:44 ` Jamal Hadi Salim 2013-01-13 16:41 ` Benjamin LaHaise 2013-01-13 16:57 ` Eric Dumazet 2013-01-13 17:25 ` Jamal Hadi Salim 2013-01-14 5:40 ` Eric Dumazet 2013-01-13 17:46 ` [PATCH net-next] ifb: dont hard code inet_net use Eric Dumazet 2013-01-14 13:13 ` Jamal Hadi Salim 2013-01-14 15:15 ` [PATCH net-next] pkt_sched: namespace aware act_mirred Benjamin LaHaise 2013-01-14 20:10 ` David Miller 2013-01-14 20:13 ` [PATCH net-next] ifb: dont hard code inet_net use David Miller 2013-01-13 14:59 ` [PATCH net-next] pkt_sched: namespace aware ifb Benjamin LaHaise 2013-01-13 16:35 ` Eric Dumazet 2013-01-13 11:01 ` [RFC davem] revert: net: Make skb->skb_iif always track skb->dev Oliver Hartkopp 2013-01-13 13:20 ` David Miller
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).