| Message ID | 4F75CC63.10405@googlemail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2012-03-30 at 17:08 +0200, Jan Seiffert wrote: > The function is renamed to make it a little more clear what it does. > It is not added to any .h because it is not for general consumption, only for > bpf internal use (and so by the jits). > > Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com> > Missing "---" line separator (check Documentation/SubmittingPatches line 490) You can check http://patchwork.ozlabs.org/patch/149683/ and see there is a problem, compared to http://patchwork.ozlabs.org/patch/149441/ for example > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -40,8 +40,12 @@ > #include <linux/reciprocal_div.h> > #include <linux/ratelimit.h> > > -/* No hurry in this branch */ > -static void *__load_pointer(const struct sk_buff *skb, int k, unsigned int size) > +/* > + * No hurry in this branch > + * > + * Exported for the bpf jit load helper. > + */ Seems good to me, maybe add a strong warning in the comment to say that function prototype can NOT change without major surgery in ASM files, since assembler wont catch the prototype change for us. Acked-by: Eric Dumazet <eric.dumazet@gmail.com> -- 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
> The function is renamed to make it a little more clear what it does. > It is not added to any .h because it is not for general > consumption, only for bpf internal use (and so by the jits). I'd have thought it better to put in into a bfp_internal.h (or similar) with a big warning there about the asm users. Possibly even worth adding some other defs that the asm files will need (if there are any). David -- 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
David Laight schrieb: > >> The function is renamed to make it a little more clear what it does. >> It is not added to any .h because it is not for general >> consumption, only for bpf internal use (and so by the jits). > > I'd have thought it better to put in into a bfp_internal.h > (or similar) with a big warning there about the asm users. > Hmmm, OK, where would i put the .h? Right there under ./include/linux/? > Possibly even worth adding some other defs that the asm > files will need (if there are any). > There is at least one other define, the lowest negative address range, but it would be a copy of the same define in filter.h, or i would have to massage filter.h to make it fit for inclusion by assembly. So I'm unsure how to proceed. > David > Greetings Jan
From: Jan Seiffert <kaffeemonster@googlemail.com> Date: Fri, 30 Mar 2012 17:08:19 +0200 > The function is renamed to make it a little more clear what it does. > It is not added to any .h because it is not for general consumption, only for > bpf internal use (and so by the jits). > > Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com> Applied but with comment formatting fixed up: > +/* > + * No hurry in this branch > + * > + * Exported for the bpf jit load helper. > + */ Like this: > +/* No hurry in this branch > + * > + * Exported for the bpf jit load helper. > + */ -- 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
David Miller schrieb: > From: Jan Seiffert <kaffeemonster@googlemail.com> > Date: Fri, 30 Mar 2012 17:08:19 +0200 > >> The function is renamed to make it a little more clear what it does. >> It is not added to any .h because it is not for general consumption, only for >> bpf internal use (and so by the jits). >> >> Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com> > > Applied but with comment formatting fixed up: > >> +/* >> + * No hurry in this branch >> + * >> + * Exported for the bpf jit load helper. >> + */ > > Like this: > >> +/* No hurry in this branch >> + * >> + * Exported for the bpf jit load helper. >> + */ > Thanks David! I was planning on a V5 for the little things i did wrong (wrong patch separator, etc.) but wanted to wait a little more for more feedback. Is that the preferred comment style in net/ ? Coding style says first line empty. Or just to match the rest of the file? Greetings Jan -- 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
From: Jan Seiffert <kaffeemonster@googlemail.com> Date: Wed, 4 Apr 2012 00:26:25 +0200 > Is that the preferred comment style in net/ ? Coding style says > first line empty. Or just to match the rest of the file? I'm not getting into this discussion right now, sorry. -- 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
David Miller schrieb: > From: Jan Seiffert <kaffeemonster@googlemail.com> > Date: Wed, 4 Apr 2012 00:26:25 +0200 > >> Is that the preferred comment style in net/ ? Coding style says >> first line empty. Or just to match the rest of the file? > > I'm not getting into this discussion right now, sorry. > Oh, sorry, i guess you misunderstood me. I didn't want to discuss anything, I'm fine either way, i just wanted to know what's the rule, so i can take notes and prevent the same error in the future. Greetings Jan -- 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
--- a/net/core/filter.c +++ b/net/core/filter.c @@ -40,8 +40,12 @@ #include <linux/reciprocal_div.h> #include <linux/ratelimit.h> -/* No hurry in this branch */ -static void *__load_pointer(const struct sk_buff *skb, int k, unsigned int size) +/* + * No hurry in this branch + * + * Exported for the bpf jit load helper. + */ +void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size) { u8 *ptr = NULL; @@ -60,7 +64,7 @@ static inline void *load_pointer(const struct sk_buff *skb, int k, { if (k >= 0) return skb_header_pointer(skb, k, size, buffer); - return __load_pointer(skb, k, size); + return bpf_internal_load_pointer_neg_helper(skb, k, size); } /**