| Message ID | 1442340563-40106-2-git-send-email-pshelar@nicira.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Sep 15, 2015 at 11:09 AM, Pravin B Shelar <pshelar@nicira.com> wrote: > Signed-off-by: Pravin B Shelar <pshelar@nicira.com> What about this commit? openvswitch: allocate nr_node_ids flow_stats instead of num_possible_nodes > diff --git a/datapath/linux/compat/socket.c b/datapath/linux/compat/socket.c > new file mode 100644 > index 0000000..776f9e2 > --- /dev/null > +++ b/datapath/linux/compat/socket.c > +#ifndef HAVE_SOCK_CREATE_KERN_NET > +int ovs_sock_create_kern(struct net *net, int family, int type, int protocol, struct socket **res) > +{ > + int err; > + > + err = sock_create_kern(family, type, protocol, res); > + if (err < 0) > + return err; > + > + sk_change_net((*res)->sk, net); > + return err; > +} > + > +void ovs_sock_release(struct socket *sock) > +{ > + sk_release_kernel(sock->sk); > +} > +#endif Is there a reason we can't use a macro and then keep the original name? I'm also not sure that we need to put this in a new .c file but that's more just personal opinion.
On Wed, Sep 16, 2015 at 3:00 PM, Jesse Gross <jesse@nicira.com> wrote: > On Tue, Sep 15, 2015 at 11:09 AM, Pravin B Shelar <pshelar@nicira.com> wrote: >> Signed-off-by: Pravin B Shelar <pshelar@nicira.com> > > What about this commit? > openvswitch: allocate nr_node_ids flow_stats instead of num_possible_nodes > ok, I will send separate patch. >> diff --git a/datapath/linux/compat/socket.c b/datapath/linux/compat/socket.c >> new file mode 100644 >> index 0000000..776f9e2 >> --- /dev/null >> +++ b/datapath/linux/compat/socket.c >> +#ifndef HAVE_SOCK_CREATE_KERN_NET >> +int ovs_sock_create_kern(struct net *net, int family, int type, int protocol, struct socket **res) >> +{ >> + int err; >> + >> + err = sock_create_kern(family, type, protocol, res); >> + if (err < 0) >> + return err; >> + >> + sk_change_net((*res)->sk, net); >> + return err; >> +} >> + >> +void ovs_sock_release(struct socket *sock) >> +{ >> + sk_release_kernel(sock->sk); >> +} >> +#endif > > Is there a reason we can't use a macro and then keep the original > name? I'm also not sure that we need to put this in a new .c file but > that's more just personal opinion. I also likes to define it in header but there is issue with it. I need to call sk_change_net() which is defined in sock.h. I do not want to include header from different compat header file. I causes all sorts of header dependencies. I changed name since it is no longer same API, it also calls sk_change_net().
On Thu, Sep 17, 2015 at 11:58 AM, Pravin Shelar <pshelar@nicira.com> wrote: > On Wed, Sep 16, 2015 at 3:00 PM, Jesse Gross <jesse@nicira.com> wrote: >> On Tue, Sep 15, 2015 at 11:09 AM, Pravin B Shelar <pshelar@nicira.com> wrote: >>> diff --git a/datapath/linux/compat/socket.c b/datapath/linux/compat/socket.c >>> new file mode 100644 >>> index 0000000..776f9e2 >>> --- /dev/null >>> +++ b/datapath/linux/compat/socket.c >>> +#ifndef HAVE_SOCK_CREATE_KERN_NET >>> +int ovs_sock_create_kern(struct net *net, int family, int type, int protocol, struct socket **res) >>> +{ >>> + int err; >>> + >>> + err = sock_create_kern(family, type, protocol, res); >>> + if (err < 0) >>> + return err; >>> + >>> + sk_change_net((*res)->sk, net); >>> + return err; >>> +} >>> + >>> +void ovs_sock_release(struct socket *sock) >>> +{ >>> + sk_release_kernel(sock->sk); >>> +} >>> +#endif >> >> Is there a reason we can't use a macro and then keep the original >> name? I'm also not sure that we need to put this in a new .c file but >> that's more just personal opinion. > > I also likes to define it in header but there is issue with it. I need > to call sk_change_net() which is defined in sock.h. I do not want to > include header from different compat header file. I causes all sorts > of header dependencies. OK. > I changed name since it is no longer same API, it also calls sk_change_net(). But isn't it trying to approximate the semantics of the current version of sock_create_kern()?
On Thu, Sep 17, 2015 at 1:08 PM, Jesse Gross <jesse@nicira.com> wrote: > On Thu, Sep 17, 2015 at 11:58 AM, Pravin Shelar <pshelar@nicira.com> wrote: >> On Wed, Sep 16, 2015 at 3:00 PM, Jesse Gross <jesse@nicira.com> wrote: >>> On Tue, Sep 15, 2015 at 11:09 AM, Pravin B Shelar <pshelar@nicira.com> wrote: >>>> diff --git a/datapath/linux/compat/socket.c b/datapath/linux/compat/socket.c >>>> new file mode 100644 >>>> index 0000000..776f9e2 >>>> --- /dev/null >>>> +++ b/datapath/linux/compat/socket.c >>>> +#ifndef HAVE_SOCK_CREATE_KERN_NET >>>> +int ovs_sock_create_kern(struct net *net, int family, int type, int protocol, struct socket **res) >>>> +{ >>>> + int err; >>>> + >>>> + err = sock_create_kern(family, type, protocol, res); >>>> + if (err < 0) >>>> + return err; >>>> + >>>> + sk_change_net((*res)->sk, net); >>>> + return err; >>>> +} >>>> + >>>> +void ovs_sock_release(struct socket *sock) >>>> +{ >>>> + sk_release_kernel(sock->sk); >>>> +} >>>> +#endif >>> >>> Is there a reason we can't use a macro and then keep the original >>> name? I'm also not sure that we need to put this in a new .c file but >>> that's more just personal opinion. >> >> I also likes to define it in header but there is issue with it. I need >> to call sk_change_net() which is defined in sock.h. I do not want to >> include header from different compat header file. I causes all sorts >> of header dependencies. > > OK. > >> I changed name since it is no longer same API, it also calls sk_change_net(). > > But isn't it trying to approximate the semantics of the current > version of sock_create_kern()? OK, Since the function arguments are different we do not need to worry about this API getting automatically used some where else in the code. I will add function name macro.
On Thu, Sep 17, 2015 at 11:58 AM, Pravin Shelar <pshelar@nicira.com> wrote: > On Wed, Sep 16, 2015 at 3:00 PM, Jesse Gross <jesse@nicira.com> wrote: >> On Tue, Sep 15, 2015 at 11:09 AM, Pravin B Shelar <pshelar@nicira.com> wrote: >>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com> >> >> What about this commit? >> openvswitch: allocate nr_node_ids flow_stats instead of num_possible_nodes >> > ok, I will send separate patch. Actually this is already backported.
On Thu, Sep 17, 2015 at 2:49 PM, Pravin Shelar <pshelar@nicira.com> wrote: > On Thu, Sep 17, 2015 at 11:58 AM, Pravin Shelar <pshelar@nicira.com> wrote: >> On Wed, Sep 16, 2015 at 3:00 PM, Jesse Gross <jesse@nicira.com> wrote: >>> On Tue, Sep 15, 2015 at 11:09 AM, Pravin B Shelar <pshelar@nicira.com> wrote: >>>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com> >>> >>> What about this commit? >>> openvswitch: allocate nr_node_ids flow_stats instead of num_possible_nodes >>> >> ok, I will send separate patch. > > Actually this is already backported. It is? It looks like master + these patches calls num_possible_nodes() in kmem_cache_create() in flow_table.c.
On Thu, Sep 17, 2015 at 5:49 PM, Jesse Gross <jesse@nicira.com> wrote: > On Thu, Sep 17, 2015 at 2:49 PM, Pravin Shelar <pshelar@nicira.com> wrote: >> On Thu, Sep 17, 2015 at 11:58 AM, Pravin Shelar <pshelar@nicira.com> wrote: >>> On Wed, Sep 16, 2015 at 3:00 PM, Jesse Gross <jesse@nicira.com> wrote: >>>> On Tue, Sep 15, 2015 at 11:09 AM, Pravin B Shelar <pshelar@nicira.com> wrote: >>>>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com> >>>> >>>> What about this commit? >>>> openvswitch: allocate nr_node_ids flow_stats instead of num_possible_nodes >>>> >>> ok, I will send separate patch. >> >> Actually this is already backported. > > It is? It looks like master + these patches calls num_possible_nodes() > in kmem_cache_create() in flow_table.c. right, I was looking at local changes. I have sent out patch.
diff --git a/.travis.yml b/.travis.yml index f4b9188..8126180 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,8 +12,8 @@ env: - TESTSUITE=1 KERNEL=3.18.1 - TESTSUITE=1 OPTS="--enable-shared" - BUILD_ENV="-m32" OPTS="--disable-ssl" + - KERNEL=4.2 - KERNEL=4.1.6 - - KERNEL=4.0.9 - KERNEL=3.17.7 DPDK=1 - KERNEL=3.17.7 DPDK=1 OPTS="--enable-shared" - KERNEL=3.17.7 diff --git a/FAQ.md b/FAQ.md index cea517d..d11c493 100644 --- a/FAQ.md +++ b/FAQ.md @@ -156,7 +156,7 @@ A: The following table lists the Linux kernel versions against which the | 2.1.x | 2.6.32 to 3.11 | 2.3.x | 2.6.32 to 3.14 | 2.4.x | 2.6.32 to 4.0 -| 2.5.x | 2.6.32 to 4.1 +| 2.5.x | 2.6.32 to 4.2 Open vSwitch userspace should also work with the Linux kernel module built into Linux 3.3 and later. diff --git a/acinclude.m4 b/acinclude.m4 index 229f900..b246dc7 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [ AC_MSG_RESULT([$kversion]) if test "$version" -ge 4; then - if test "$version" = 4 && test "$patchlevel" -le 1; then + if test "$version" = 4 && test "$patchlevel" -le 2; then : # Linux 4.x else - AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.1.x is not supported (please refer to the FAQ for advice)]) + AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.2.x is not supported (please refer to the FAQ for advice)]) fi elif test "$version" = 3; then : # Linux 3.x @@ -331,6 +331,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ [OVS_DEFINE([HAVE_INET_GET_LOCAL_PORT_RANGE_USING_NET])]) OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_is_fragment]) + OVS_GREP_IFELSE([$KSRC/include/linux/net.h], [sock_create_kern.*net], + [OVS_DEFINE([HAVE_SOCK_CREATE_KERN_NET])]) OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [dev_disable_lro]) OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [dev_get_stats]) OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [dev_get_by_index_rcu]) @@ -415,6 +417,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_FIND_FIELD_IFELSE([$KSRC/include/net/genetlink.h], [genl_multicast_group], [id]) + OVS_GREP_IFELSE([$KSRC/include/net/geneve.h], [geneve_hdr]) OVS_GREP_IFELSE([$KSRC/include/net/gre.h], [gre_cisco_register]) OVS_GREP_IFELSE([$KSRC/include/net/ipv6.h], [IP6_FH_F_SKIP_RH]) diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk index be3a8d8..96c3d55 100644 --- a/datapath/linux/Modules.mk +++ b/datapath/linux/Modules.mk @@ -12,6 +12,7 @@ openvswitch_sources += \ linux/compat/net_namespace.c \ linux/compat/reciprocal_div.c \ linux/compat/skbuff-openvswitch.c \ + linux/compat/socket.c \ linux/compat/stt.c \ linux/compat/udp.c \ linux/compat/udp_tunnel.c \ diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c index 8e80180..85cf95f 100644 --- a/datapath/linux/compat/geneve.c +++ b/datapath/linux/compat/geneve.c @@ -56,11 +56,6 @@ #include "compat.h" #include "gso.h" -static inline struct genevehdr *geneve_hdr(const struct sk_buff *skb) -{ - return (struct genevehdr *)(udp_hdr(skb) + 1); -} - static void geneve_build_header(struct genevehdr *geneveh, __be16 tun_flags, u8 vni[3], u8 options_len, u8 *options) diff --git a/datapath/linux/compat/include/linux/net.h b/datapath/linux/compat/include/linux/net.h index 9c94745..36736de 100644 --- a/datapath/linux/compat/include/linux/net.h +++ b/datapath/linux/compat/include/linux/net.h @@ -52,4 +52,12 @@ bool rpl___net_get_random_once(void *buf, int nbytes, bool *done, }) #endif +#ifndef HAVE_SOCK_CREATE_KERN_NET +int ovs_sock_create_kern(struct net *net, int family, int type, int protocol, struct socket **res); +void ovs_sock_release(struct socket *sock); +#else +#define ovs_sock_create_kern sock_create_kern +#define ovs_sock_release sock_release +#endif + #endif diff --git a/datapath/linux/compat/include/net/geneve.h b/datapath/linux/compat/include/net/geneve.h index 58f5def..4f250c2 100644 --- a/datapath/linux/compat/include/net/geneve.h +++ b/datapath/linux/compat/include/net/geneve.h @@ -101,4 +101,11 @@ int rpl_geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt, #endif /* kernel < 4.0 */ +#ifndef HAVE_GENEVE_HDR +static inline struct genevehdr *geneve_hdr(const struct sk_buff *skb) +{ + return (struct genevehdr *)(udp_hdr(skb) + 1); +} +#endif + #endif /*ifdef__NET_GENEVE_WRAPPER_H */ diff --git a/datapath/linux/compat/socket.c b/datapath/linux/compat/socket.c new file mode 100644 index 0000000..776f9e2 --- /dev/null +++ b/datapath/linux/compat/socket.c @@ -0,0 +1,30 @@ +#include <linux/module.h> +#include <linux/errno.h> +#include <linux/socket.h> +#include <linux/udp.h> +#include <linux/types.h> +#include <linux/kernel.h> +#include <net/ip_tunnels.h> +#include <net/udp.h> +#include <net/udp_tunnel.h> +#include <net/net_namespace.h> + + +#ifndef HAVE_SOCK_CREATE_KERN_NET +int ovs_sock_create_kern(struct net *net, int family, int type, int protocol, struct socket **res) +{ + int err; + + err = sock_create_kern(family, type, protocol, res); + if (err < 0) + return err; + + sk_change_net((*res)->sk, net); + return err; +} + +void ovs_sock_release(struct socket *sock) +{ + sk_release_kernel(sock->sk); +} +#endif diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c index e27cedf..654dfee 100644 --- a/datapath/linux/compat/stt.c +++ b/datapath/linux/compat/stt.c @@ -21,6 +21,7 @@ #include <linux/list.h> #include <linux/log2.h> #include <linux/module.h> +#include <linux/net.h> #include <linux/netfilter.h> #include <linux/percpu.h> #include <linux/skbuff.h> @@ -1251,7 +1252,7 @@ drop: static void tcp_sock_release(struct socket *sock) { kernel_sock_shutdown(sock, SHUT_RDWR); - sk_release_kernel(sock->sk); + ovs_sock_release(sock); } static int tcp_sock_create4(struct net *net, __be16 port, @@ -1261,12 +1262,10 @@ static int tcp_sock_create4(struct net *net, __be16 port, struct socket *sock = NULL; int err; - err = sock_create_kern(AF_INET, SOCK_STREAM, IPPROTO_TCP, &sock); + err = ovs_sock_create_kern(net, AF_INET, SOCK_STREAM, IPPROTO_TCP, &sock); if (err < 0) goto error; - sk_change_net(sock->sk, net); - memset(&tcp_addr, 0, sizeof(tcp_addr)); tcp_addr.sin_family = AF_INET; tcp_addr.sin_addr.s_addr = htonl(INADDR_ANY); diff --git a/datapath/linux/compat/udp_tunnel.c b/datapath/linux/compat/udp_tunnel.c index 680fd83..b6c5bad 100644 --- a/datapath/linux/compat/udp_tunnel.c +++ b/datapath/linux/compat/udp_tunnel.c @@ -90,7 +90,7 @@ int rpl_udp_sock_create(struct net *net, struct udp_port_cfg *cfg, error: if (sock) { kernel_sock_shutdown(sock, SHUT_RDWR); - sk_release_kernel(sock->sk); + ovs_sock_release(sock); } *sockp = NULL; return err; @@ -168,7 +168,7 @@ void rpl_udp_tunnel_sock_release(struct socket *sock) { rcu_assign_sk_user_data(sock->sk, NULL); kernel_sock_shutdown(sock, SHUT_RDWR); - sk_release_kernel(sock->sk); + ovs_sock_release(sock); } EXPORT_SYMBOL_GPL(rpl_udp_tunnel_sock_release); diff --git a/datapath/vport-geneve.c b/datapath/vport-geneve.c index 2d7a6b3..4ab224d 100644 --- a/datapath/vport-geneve.c +++ b/datapath/vport-geneve.c @@ -46,11 +46,6 @@ static inline struct geneve_port *geneve_vport(const struct vport *vport) return vport_priv(vport); } -static inline struct genevehdr *geneve_hdr(const struct sk_buff *skb) -{ - return (struct genevehdr *)(udp_hdr(skb) + 1); -} - /* Convert 64 bit tunnel ID to 24 bit VNI. */ static void tunnel_id_to_vni(__be64 tun_id, __u8 *vni) {
Signed-off-by: Pravin B Shelar <pshelar@nicira.com> --- .travis.yml | 2 +- FAQ.md | 2 +- acinclude.m4 | 7 ++++- datapath/linux/Modules.mk | 1 + datapath/linux/compat/geneve.c | 5 ---- datapath/linux/compat/include/linux/net.h | 8 +++++++ datapath/linux/compat/include/net/geneve.h | 7 ++++++ datapath/linux/compat/socket.c | 30 ++++++++++++++++++++++++++++ datapath/linux/compat/stt.c | 7 ++--- datapath/linux/compat/udp_tunnel.c | 4 +- datapath/vport-geneve.c | 5 ---- 11 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 datapath/linux/compat/socket.c