Re: linux-next: Tree for March 10 (crypto & NLATTR)

Re: linux-next: Tree for March 10 (crypto & NLATTR)

[Randy Dunlap]

Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20090306:
> 
> 
> The driver-core tree gained a build failure due to a conflict with the
> crypto tree.  I have applied a patch to the crypto tree for today.

I had several (4 of 50) randconfig builds fail with:

lib/built-in.o: In function `__nla_reserve_nohdr':
(.text+0xd08d): undefined reference to `skb_put'
lib/built-in.o: In function `__nla_reserve':
(.text+0xd121): undefined reference to `skb_put'
lib/built-in.o: In function `nla_append':
(.text+0xd493): undefined reference to `skb_put'

which happens with CONFIG_NET=n, CONFIG_CRYPTO=y, CONFIG_CRYPTO_ZLIB=[my].

CRYPTO_ZLIB selects NLATTR, but obviously the build of nlattr.c fails
when CONFIG_NET=n.  Should CRYPTO_ZLIB depend on NET?
Please don't say that CRYPTO_ZLIB should select NET.


-- 
~Randy

Re: linux-next: Tree for March 10 (crypto & NLATTR)

[Geert Uytterhoeven]

On Tue, Mar 10, 2009 at 19:57, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> Stephen Rothwell wrote:
>> Changes since 20090306:
>>
>>
>> The driver-core tree gained a build failure due to a conflict with the
>> crypto tree.  I have applied a patch to the crypto tree for today.
>
> I had several (4 of 50) randconfig builds fail with:
>
> lib/built-in.o: In function `__nla_reserve_nohdr':
> (.text+0xd08d): undefined reference to `skb_put'
> lib/built-in.o: In function `__nla_reserve':
> (.text+0xd121): undefined reference to `skb_put'
> lib/built-in.o: In function `nla_append':
> (.text+0xd493): undefined reference to `skb_put'
>
> which happens with CONFIG_NET=n, CONFIG_CRYPTO=y, CONFIG_CRYPTO_ZLIB=[my].
>
> CRYPTO_ZLIB selects NLATTR, but obviously the build of nlattr.c fails
> when CONFIG_NET=n.  Should CRYPTO_ZLIB depend on NET?
> Please don't say that CRYPTO_ZLIB should select NET.

Bummer, my fault (commit e9cc8bddaea3944fabfebb968bc88d603239beed,
netlink: Move netlink attribute parsing support to lib).

Obviously I was only worried about crypto/zlib.c needing nlattr.c
without pulling in the whole networking code, not about nlattr.c
itself needing networking functionality. But still, how could I have
missed this compile failure?

Does it sound sane to protect the routines that do call skb_put() by
#ifdef CONFIG_NET?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: linux-next: Tree for March 10 (crypto & NLATTR)

[Randy Dunlap]

Geert Uytterhoeven wrote:
> On Tue, Mar 10, 2009 at 19:57, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>> Stephen Rothwell wrote:
>>> Changes since 20090306:
>>>
>>>
>>> The driver-core tree gained a build failure due to a conflict with the
>>> crypto tree.  I have applied a patch to the crypto tree for today.
>> I had several (4 of 50) randconfig builds fail with:
>>
>> lib/built-in.o: In function `__nla_reserve_nohdr':
>> (.text+0xd08d): undefined reference to `skb_put'
>> lib/built-in.o: In function `__nla_reserve':
>> (.text+0xd121): undefined reference to `skb_put'
>> lib/built-in.o: In function `nla_append':
>> (.text+0xd493): undefined reference to `skb_put'
>>
>> which happens with CONFIG_NET=n, CONFIG_CRYPTO=y, CONFIG_CRYPTO_ZLIB=[my].
>>
>> CRYPTO_ZLIB selects NLATTR, but obviously the build of nlattr.c fails
>> when CONFIG_NET=n.  Should CRYPTO_ZLIB depend on NET?
>> Please don't say that CRYPTO_ZLIB should select NET.
> 
> Bummer, my fault (commit e9cc8bddaea3944fabfebb968bc88d603239beed,
> netlink: Move netlink attribute parsing support to lib).
> 
> Obviously I was only worried about crypto/zlib.c needing nlattr.c
> without pulling in the whole networking code, not about nlattr.c
> itself needing networking functionality. But still, how could I have
> missed this compile failure?
> 
> Does it sound sane to protect the routines that do call skb_put() by
> #ifdef CONFIG_NET?

I'll have to let David or Herbert answer that.  From my quick look
at the code, I don't see much use for nlattr.c when CONFIG_NET
is not enabled.

-- 
~Randy

Re: linux-next: Tree for March 10 (crypto & NLATTR)

[David Miller]

From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Tue, 10 Mar 2009 13:08:31 -0700

> I'll have to let David or Herbert answer that.  From my quick look
> at the code, I don't see much use for nlattr.c when CONFIG_NET
> is not enabled.

We want to use the netlink attribute parsers even in non-networking
code, that's what he's trying to do here.

Re: linux-next: Tree for March 10 (crypto & NLATTR)

[Herbert Xu]

On Tue, Mar 10, 2009 at 01:17:00PM -0700, David Miller wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
> Date: Tue, 10 Mar 2009 13:08:31 -0700
> 
> > I'll have to let David or Herbert answer that.  From my quick look
> > at the code, I don't see much use for nlattr.c when CONFIG_NET
> > is not enabled.
> 
> We want to use the netlink attribute parsers even in non-networking
> code, that's what he's trying to do here.

OK the nlattr construction code really wants to depend on NET
because they're skb-oriented.  We could either move them back
or just wrap them in ifdef CONFIG_NET.

I think the latter is probably the simplest.

How does this patch look? And Randy, does it fix the problem for
you?

nlattr: Fix build error with NET off

We moved the netlink attribute support from net to lib in order
for it to be available for general consumption.  However, parts
of the code (the bits that we don't need :) really depends on
NET because the target object is sk_buff.

This patch fixes this by wrapping them in CONFIG_NET.

Some EXPORTs have been moved to make this work.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/lib/nlattr.c b/lib/nlattr.c
index 56c3ce7..80009a2 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -281,6 +281,7 @@ int nla_strcmp(const struct nlattr *nla, const char *str)
 	return d;
 }
 
+#ifdef CONFIG_NET
 /**
  * __nla_reserve - reserve room for attribute on the skb
  * @skb: socket buffer to reserve room on
@@ -305,6 +306,7 @@ struct nlattr *__nla_reserve(struct sk_buff *skb, int attrtype, int attrlen)
 
 	return nla;
 }
+EXPORT_SYMBOL(__nla_reserve);
 
 /**
  * __nla_reserve_nohdr - reserve room for attribute without header
@@ -325,6 +327,7 @@ void *__nla_reserve_nohdr(struct sk_buff *skb, int attrlen)
 
 	return start;
 }
+EXPORT_SYMBOL(__nla_reserve_nohdr);
 
 /**
  * nla_reserve - reserve room for attribute on the skb
@@ -345,6 +348,7 @@ struct nlattr *nla_reserve(struct sk_buff *skb, int attrtype, int attrlen)
 
 	return __nla_reserve(skb, attrtype, attrlen);
 }
+EXPORT_SYMBOL(nla_reserve);
 
 /**
  * nla_reserve_nohdr - reserve room for attribute without header
@@ -363,6 +367,7 @@ void *nla_reserve_nohdr(struct sk_buff *skb, int attrlen)
 
 	return __nla_reserve_nohdr(skb, attrlen);
 }
+EXPORT_SYMBOL(nla_reserve_nohdr);
 
 /**
  * __nla_put - Add a netlink attribute to a socket buffer
@@ -382,6 +387,7 @@ void __nla_put(struct sk_buff *skb, int attrtype, int attrlen,
 	nla = __nla_reserve(skb, attrtype, attrlen);
 	memcpy(nla_data(nla), data, attrlen);
 }
+EXPORT_SYMBOL(__nla_put);
 
 /**
  * __nla_put_nohdr - Add a netlink attribute without header
@@ -399,6 +405,7 @@ void __nla_put_nohdr(struct sk_buff *skb, int attrlen, const void *data)
 	start = __nla_reserve_nohdr(skb, attrlen);
 	memcpy(start, data, attrlen);
 }
+EXPORT_SYMBOL(__nla_put_nohdr);
 
 /**
  * nla_put - Add a netlink attribute to a socket buffer
@@ -418,6 +425,7 @@ int nla_put(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
 	__nla_put(skb, attrtype, attrlen, data);
 	return 0;
 }
+EXPORT_SYMBOL(nla_put);
 
 /**
  * nla_put_nohdr - Add a netlink attribute without header
@@ -436,6 +444,7 @@ int nla_put_nohdr(struct sk_buff *skb, int attrlen, const void *data)
 	__nla_put_nohdr(skb, attrlen, data);
 	return 0;
 }
+EXPORT_SYMBOL(nla_put_nohdr);
 
 /**
  * nla_append - Add a netlink attribute without header or padding
@@ -454,20 +463,13 @@ int nla_append(struct sk_buff *skb, int attrlen, const void *data)
 	memcpy(skb_put(skb, attrlen), data, attrlen);
 	return 0;
 }
+EXPORT_SYMBOL(nla_append);
+#endif
 
 EXPORT_SYMBOL(nla_validate);
 EXPORT_SYMBOL(nla_parse);
 EXPORT_SYMBOL(nla_find);
 EXPORT_SYMBOL(nla_strlcpy);
-EXPORT_SYMBOL(__nla_reserve);
-EXPORT_SYMBOL(__nla_reserve_nohdr);
-EXPORT_SYMBOL(nla_reserve);
-EXPORT_SYMBOL(nla_reserve_nohdr);
-EXPORT_SYMBOL(__nla_put);
-EXPORT_SYMBOL(__nla_put_nohdr);
-EXPORT_SYMBOL(nla_put);
-EXPORT_SYMBOL(nla_put_nohdr);
 EXPORT_SYMBOL(nla_memcpy);
 EXPORT_SYMBOL(nla_memcmp);
 EXPORT_SYMBOL(nla_strcmp);
-EXPORT_SYMBOL(nla_append);

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: Tree for March 10 (crypto & NLATTR)

[Geert Uytterhoeven]

On Wed, 11 Mar 2009, Herbert Xu wrote:
> On Tue, Mar 10, 2009 at 01:17:00PM -0700, David Miller wrote:
> > From: Randy Dunlap <randy.dunlap@oracle.com>
> > Date: Tue, 10 Mar 2009 13:08:31 -0700
> > 
> > > I'll have to let David or Herbert answer that.  From my quick look
> > > at the code, I don't see much use for nlattr.c when CONFIG_NET
> > > is not enabled.
> > 
> > We want to use the netlink attribute parsers even in non-networking
> > code, that's what he's trying to do here.
> 
> OK the nlattr construction code really wants to depend on NET
> because they're skb-oriented.  We could either move them back
> or just wrap them in ifdef CONFIG_NET.
> 
> I think the latter is probably the simplest.
> 
> How does this patch look? And Randy, does it fix the problem for
> you?
> 
> nlattr: Fix build error with NET off
> 
> We moved the netlink attribute support from net to lib in order
> for it to be available for general consumption.  However, parts
> of the code (the bits that we don't need :) really depends on
> NET because the target object is sk_buff.
> 
> This patch fixes this by wrapping them in CONFIG_NET.
> 
> Some EXPORTs have been moved to make this work.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks for taking care of this!

Tested-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 56c3ce7..80009a2 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -281,6 +281,7 @@ int nla_strcmp(const struct nlattr *nla, const char *str)
>  	return d;
>  }
>  
> +#ifdef CONFIG_NET
>  /**
>   * __nla_reserve - reserve room for attribute on the skb
>   * @skb: socket buffer to reserve room on
> @@ -305,6 +306,7 @@ struct nlattr *__nla_reserve(struct sk_buff *skb, int attrtype, int attrlen)
>  
>  	return nla;
>  }
> +EXPORT_SYMBOL(__nla_reserve);
>  
>  /**
>   * __nla_reserve_nohdr - reserve room for attribute without header
> @@ -325,6 +327,7 @@ void *__nla_reserve_nohdr(struct sk_buff *skb, int attrlen)
>  
>  	return start;
>  }
> +EXPORT_SYMBOL(__nla_reserve_nohdr);
>  
>  /**
>   * nla_reserve - reserve room for attribute on the skb
> @@ -345,6 +348,7 @@ struct nlattr *nla_reserve(struct sk_buff *skb, int attrtype, int attrlen)
>  
>  	return __nla_reserve(skb, attrtype, attrlen);
>  }
> +EXPORT_SYMBOL(nla_reserve);
>  
>  /**
>   * nla_reserve_nohdr - reserve room for attribute without header
> @@ -363,6 +367,7 @@ void *nla_reserve_nohdr(struct sk_buff *skb, int attrlen)
>  
>  	return __nla_reserve_nohdr(skb, attrlen);
>  }
> +EXPORT_SYMBOL(nla_reserve_nohdr);
>  
>  /**
>   * __nla_put - Add a netlink attribute to a socket buffer
> @@ -382,6 +387,7 @@ void __nla_put(struct sk_buff *skb, int attrtype, int attrlen,
>  	nla = __nla_reserve(skb, attrtype, attrlen);
>  	memcpy(nla_data(nla), data, attrlen);
>  }
> +EXPORT_SYMBOL(__nla_put);
>  
>  /**
>   * __nla_put_nohdr - Add a netlink attribute without header
> @@ -399,6 +405,7 @@ void __nla_put_nohdr(struct sk_buff *skb, int attrlen, const void *data)
>  	start = __nla_reserve_nohdr(skb, attrlen);
>  	memcpy(start, data, attrlen);
>  }
> +EXPORT_SYMBOL(__nla_put_nohdr);
>  
>  /**
>   * nla_put - Add a netlink attribute to a socket buffer
> @@ -418,6 +425,7 @@ int nla_put(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
>  	__nla_put(skb, attrtype, attrlen, data);
>  	return 0;
>  }
> +EXPORT_SYMBOL(nla_put);
>  
>  /**
>   * nla_put_nohdr - Add a netlink attribute without header
> @@ -436,6 +444,7 @@ int nla_put_nohdr(struct sk_buff *skb, int attrlen, const void *data)
>  	__nla_put_nohdr(skb, attrlen, data);
>  	return 0;
>  }
> +EXPORT_SYMBOL(nla_put_nohdr);
>  
>  /**
>   * nla_append - Add a netlink attribute without header or padding
> @@ -454,20 +463,13 @@ int nla_append(struct sk_buff *skb, int attrlen, const void *data)
>  	memcpy(skb_put(skb, attrlen), data, attrlen);
>  	return 0;
>  }
> +EXPORT_SYMBOL(nla_append);
> +#endif
>  
>  EXPORT_SYMBOL(nla_validate);
>  EXPORT_SYMBOL(nla_parse);
>  EXPORT_SYMBOL(nla_find);
>  EXPORT_SYMBOL(nla_strlcpy);
> -EXPORT_SYMBOL(__nla_reserve);
> -EXPORT_SYMBOL(__nla_reserve_nohdr);
> -EXPORT_SYMBOL(nla_reserve);
> -EXPORT_SYMBOL(nla_reserve_nohdr);
> -EXPORT_SYMBOL(__nla_put);
> -EXPORT_SYMBOL(__nla_put_nohdr);
> -EXPORT_SYMBOL(nla_put);
> -EXPORT_SYMBOL(nla_put_nohdr);
>  EXPORT_SYMBOL(nla_memcpy);
>  EXPORT_SYMBOL(nla_memcmp);
>  EXPORT_SYMBOL(nla_strcmp);
> -EXPORT_SYMBOL(nla_append);

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: linux-next: Tree for March 10 (crypto & NLATTR)

[Randy Dunlap]

Herbert Xu wrote:
> On Tue, Mar 10, 2009 at 01:17:00PM -0700, David Miller wrote:
>> From: Randy Dunlap <randy.dunlap@oracle.com>
>> Date: Tue, 10 Mar 2009 13:08:31 -0700
>>
>>> I'll have to let David or Herbert answer that.  From my quick look
>>> at the code, I don't see much use for nlattr.c when CONFIG_NET
>>> is not enabled.
>> We want to use the netlink attribute parsers even in non-networking
>> code, that's what he's trying to do here.
> 
> OK the nlattr construction code really wants to depend on NET
> because they're skb-oriented.  We could either move them back
> or just wrap them in ifdef CONFIG_NET.
> 
> I think the latter is probably the simplest.
> 
> How does this patch look? And Randy, does it fix the problem for
> you?
> 
> nlattr: Fix build error with NET off
> 
> We moved the netlink attribute support from net to lib in order
> for it to be available for general consumption.  However, parts
> of the code (the bits that we don't need :) really depends on
> NET because the target object is sk_buff.
> 
> This patch fixes this by wrapping them in CONFIG_NET.
> 
> Some EXPORTs have been moved to make this work.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
Tested-by: Randy Dunlap <randy.dunlap@oracle.com>

Thanks.

> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 56c3ce7..80009a2 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -281,6 +281,7 @@ int nla_strcmp(const struct nlattr *nla, const char *str)
>  	return d;
>  }
>  
> +#ifdef CONFIG_NET
>  /**
>   * __nla_reserve - reserve room for attribute on the skb
>   * @skb: socket buffer to reserve room on
> @@ -305,6 +306,7 @@ struct nlattr *__nla_reserve(struct sk_buff *skb, int attrtype, int attrlen)
>  
>  	return nla;
>  }
> +EXPORT_SYMBOL(__nla_reserve);
>  
>  /**
>   * __nla_reserve_nohdr - reserve room for attribute without header
> @@ -325,6 +327,7 @@ void *__nla_reserve_nohdr(struct sk_buff *skb, int attrlen)
>  
>  	return start;
>  }
> +EXPORT_SYMBOL(__nla_reserve_nohdr);
>  
>  /**
>   * nla_reserve - reserve room for attribute on the skb
> @@ -345,6 +348,7 @@ struct nlattr *nla_reserve(struct sk_buff *skb, int attrtype, int attrlen)
>  
>  	return __nla_reserve(skb, attrtype, attrlen);
>  }
> +EXPORT_SYMBOL(nla_reserve);
>  
>  /**
>   * nla_reserve_nohdr - reserve room for attribute without header
> @@ -363,6 +367,7 @@ void *nla_reserve_nohdr(struct sk_buff *skb, int attrlen)
>  
>  	return __nla_reserve_nohdr(skb, attrlen);
>  }
> +EXPORT_SYMBOL(nla_reserve_nohdr);
>  
>  /**
>   * __nla_put - Add a netlink attribute to a socket buffer
> @@ -382,6 +387,7 @@ void __nla_put(struct sk_buff *skb, int attrtype, int attrlen,
>  	nla = __nla_reserve(skb, attrtype, attrlen);
>  	memcpy(nla_data(nla), data, attrlen);
>  }
> +EXPORT_SYMBOL(__nla_put);
>  
>  /**
>   * __nla_put_nohdr - Add a netlink attribute without header
> @@ -399,6 +405,7 @@ void __nla_put_nohdr(struct sk_buff *skb, int attrlen, const void *data)
>  	start = __nla_reserve_nohdr(skb, attrlen);
>  	memcpy(start, data, attrlen);
>  }
> +EXPORT_SYMBOL(__nla_put_nohdr);
>  
>  /**
>   * nla_put - Add a netlink attribute to a socket buffer
> @@ -418,6 +425,7 @@ int nla_put(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
>  	__nla_put(skb, attrtype, attrlen, data);
>  	return 0;
>  }
> +EXPORT_SYMBOL(nla_put);
>  
>  /**
>   * nla_put_nohdr - Add a netlink attribute without header
> @@ -436,6 +444,7 @@ int nla_put_nohdr(struct sk_buff *skb, int attrlen, const void *data)
>  	__nla_put_nohdr(skb, attrlen, data);
>  	return 0;
>  }
> +EXPORT_SYMBOL(nla_put_nohdr);
>  
>  /**
>   * nla_append - Add a netlink attribute without header or padding
> @@ -454,20 +463,13 @@ int nla_append(struct sk_buff *skb, int attrlen, const void *data)
>  	memcpy(skb_put(skb, attrlen), data, attrlen);
>  	return 0;
>  }
> +EXPORT_SYMBOL(nla_append);
> +#endif
>  
>  EXPORT_SYMBOL(nla_validate);
>  EXPORT_SYMBOL(nla_parse);
>  EXPORT_SYMBOL(nla_find);
>  EXPORT_SYMBOL(nla_strlcpy);
> -EXPORT_SYMBOL(__nla_reserve);
> -EXPORT_SYMBOL(__nla_reserve_nohdr);
> -EXPORT_SYMBOL(nla_reserve);
> -EXPORT_SYMBOL(nla_reserve_nohdr);
> -EXPORT_SYMBOL(__nla_put);
> -EXPORT_SYMBOL(__nla_put_nohdr);
> -EXPORT_SYMBOL(nla_put);
> -EXPORT_SYMBOL(nla_put_nohdr);
>  EXPORT_SYMBOL(nla_memcpy);
>  EXPORT_SYMBOL(nla_memcmp);
>  EXPORT_SYMBOL(nla_strcmp);
> -EXPORT_SYMBOL(nla_append);
> 
> Thanks,


-- 
~Randy