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
