Yeah, I'll update the code accordingly and resend the whole series,
Thanks, Alex Wang, On Fri, Apr 24, 2015 at 2:36 PM, Jesse Gross <je...@nicira.com> wrote: > Sounds good to me - I see that that symbol that I pointed out > (__skb_get_hash) is defined by a macro and as a result doesn't get > picked up by you function check, whereas most cases would be. However, > still seems nice to have an explicit check for exported symbols. > > On Fri, Apr 24, 2015 at 2:28 PM, Alex Wang <al...@nicira.com> wrote: > > Thx a lot for the comments~ > > > > For the second comment, yes, Iet's make ovs_ prefix also available. > > > > For the first comment, yes, I need to add a check that grep for all > > 'EXPORT_SYMBOL_GPL' lines and make sure the symbol is either > > rpl_ or ovs_ prefixed. > > > > How about it~? > > > > Thanks, > > Alex Wang, > > > > On Fri, Apr 24, 2015 at 2:20 PM, Jesse Gross <je...@nicira.com> wrote: > >> > >> On Tue, Apr 21, 2015 at 3:20 PM, Alex Wang <al...@nicira.com> wrote: > >> > diff --git a/datapath/linux/compat/flow_dissector.c > >> > b/datapath/linux/compat/flow_dissector.c > >> > index a68f84f..c6644a5 100644 > >> > --- a/datapath/linux/compat/flow_dissector.c > >> > +++ b/datapath/linux/compat/flow_dissector.c > >> > @@ -232,4 +232,5 @@ u32 __skb_get_hash(struct sk_buff *skb) > >> > return hash; > >> > } > >> > EXPORT_SYMBOL_GPL(__skb_get_hash); > >> > >> I know the above export isn't something added by this patch but were > >> you also planning on having a check that exported symbols are prefixed > >> by rpl_? > >> > >> > diff --git a/datapath/linux/compat/include/net/udp_tunnel.h > >> > b/datapath/linux/compat/include/net/udp_tunnel.h > >> > index 6c25ca5..4c8335c 100644 > >> > --- a/datapath/linux/compat/include/net/udp_tunnel.h > >> > +++ b/datapath/linux/compat/include/net/udp_tunnel.h > >> > -void ovs_udp_gso(struct sk_buff *skb); > >> > -void ovs_udp_csum_gso(struct sk_buff *skb); > >> > +#define ovs_udp_gso rpl_ovs_udp_gso > >> > +void rpl_ovs_udp_gso(struct sk_buff *skb); > >> > +#define ovs_udp_csum_gso rpl_ovs_udp_csum_gso > >> > +void rpl_ovs_udp_csum_gso(struct sk_buff *skb); > >> > >> One thing that is a bit odd is some of these new "rpl_" functions > >> aren't actually upstream (since they are helper functions like the > >> ones above) and therefore aren't replacing anything. Since many of > >> these are already prefixed by ovs_, what if we allowed that as an > >> acceptable prefix as well for these cases? > > > > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
