[PATCH v2] rtlwifi: rtl8192cu: Remove variable self-assignment in rf.c
In _rtl92c_get_txpower_writeval_by_regulatory() the variable writeVal is assigned to itself in an if ... else statement, apparently only to document that the branch condition is handled and that a previously read value should be returned unmodified. The self-assignment causes clang to raise the following warning: drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c:304:13: error: explicitly assigning value of variable of type 'u32' (aka 'unsigned int') to itself [-Werror,-Wself-assign] writeVal = writeVal; Delete the branch with the self-assignment. Signed-off-by: Matthias Kaehlcke <m...@chromium.org> --- Changes in v2: - Delete the 'else if' branch entirely drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c index 9cff6bc4049c..cf551785eb08 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c @@ -299,9 +299,6 @@ static void _rtl92c_get_txpower_writeval_by_regulatory(struct ieee80211_hw *hw, writeVal = 0x; if (rtlpriv->dm.dynamic_txhighpower_lvl == TXHIGHPWRLEVEL_BT1) writeVal = writeVal - 0x06060606; - else if (rtlpriv->dm.dynamic_txhighpower_lvl == -TXHIGHPWRLEVEL_BT2) - writeVal = writeVal; *(p_outwriteval + rf) = writeVal; } } -- 2.16.0.rc1.238.g530d649a79-goog
Re: [PATCH] rtlwifi: rtl8192cu: Remove variable self-assignment in rf.c
El Wed, Feb 07, 2018 at 02:35:59PM -0600 Larry Finger ha dit: > On 02/07/2018 02:26 PM, Matthias Kaehlcke wrote: > > In _rtl92c_get_txpower_writeval_by_regulatory() the variable writeVal > > is assigned to itself in an if ... else statement, apparently only to > > document that the branch condition is handled and that a previously read > > value should be returned unmodified. The self-assignment causes clang to > > raise the following warning: > > > > drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c:304:13: > >error: explicitly assigning value of variable of type 'u32' > > (aka 'unsigned int') to itself [-Werror,-Wself-assign] > >writeVal = writeVal; > > > > Replace the self-assignment with a semicolon, which still serves to > > document the 'handling' of the branch condition. > > > > Signed-off-by: Matthias Kaehlcke <m...@chromium.org> > > --- > > drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c > > b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c > > index 9cff6bc4049c..4db92496c122 100644 > > --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c > > +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c > > @@ -301,7 +301,7 @@ static void > > _rtl92c_get_txpower_writeval_by_regulatory(struct ieee80211_hw *hw, > > writeVal = writeVal - 0x06060606; > > else if (rtlpriv->dm.dynamic_txhighpower_lvl == > > TXHIGHPWRLEVEL_BT2) > > - writeVal = writeVal; > > + ; > > *(p_outwriteval + rf) = writeVal; > > } > > } > > > > As the branch condition does nothing, why not remove it and save the > compiler's optimizer a bit of work? The code looks strange, but it matches > the rest of Realtek's USB drivers. Sure, I am happy to change it to whatever the authors/maintainers prefer. I'll wait a bit before respinning for if others feel strongly about keeping the branch.
[PATCH] rtlwifi: rtl8192cu: Remove variable self-assignment in rf.c
In _rtl92c_get_txpower_writeval_by_regulatory() the variable writeVal is assigned to itself in an if ... else statement, apparently only to document that the branch condition is handled and that a previously read value should be returned unmodified. The self-assignment causes clang to raise the following warning: drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c:304:13: error: explicitly assigning value of variable of type 'u32' (aka 'unsigned int') to itself [-Werror,-Wself-assign] writeVal = writeVal; Replace the self-assignment with a semicolon, which still serves to document the 'handling' of the branch condition. Signed-off-by: Matthias Kaehlcke <m...@chromium.org> --- drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c index 9cff6bc4049c..4db92496c122 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/rf.c @@ -301,7 +301,7 @@ static void _rtl92c_get_txpower_writeval_by_regulatory(struct ieee80211_hw *hw, writeVal = writeVal - 0x06060606; else if (rtlpriv->dm.dynamic_txhighpower_lvl == TXHIGHPWRLEVEL_BT2) - writeVal = writeVal; + ; *(p_outwriteval + rf) = writeVal; } } -- 2.16.0.rc1.238.g530d649a79-goog
Re: [PATCH] wimax/i2400m: Remove VLAIS
El Mon, Oct 09, 2017 at 12:41:53PM -0700 Matthias Kaehlcke ha dit:
> From: Behan Webster <beh...@converseincode.com>
>
> Convert Variable Length Array in Struct (VLAIS) to valid C by converting
> local struct definition to use a flexible array. The structure is only
> used to define a cast of a buffer so the size of the struct is not used
> to allocate storage.
>
> Signed-off-by: Behan Webster <beh...@converseincode.com>
> Signed-off-by: Mark Charebois <charl...@gmail.com>
> Suggested-by: Arnd Bergmann <a...@arndb.de>
> Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
> ---
> drivers/net/wimax/i2400m/fw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
> index c9c711dcd0e6..a89b5685e68b 100644
> --- a/drivers/net/wimax/i2400m/fw.c
> +++ b/drivers/net/wimax/i2400m/fw.c
> @@ -652,7 +652,7 @@ static int i2400m_download_chunk(struct i2400m *i2400m,
> const void *chunk,
> struct device *dev = i2400m_dev(i2400m);
> struct {
> struct i2400m_bootrom_header cmd;
> - u8 cmd_payload[chunk_len];
> + u8 cmd_payload[];
> } __packed *buf;
> struct i2400m_bootrom_header ack;
ping
any comments on this?
Re: [PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
El Tue, Oct 24, 2017 at 10:03:56AM -0700 Jakub Kicinski ha dit:
> On Tue, 24 Oct 2017 09:56:03 -0700, Matthias Kaehlcke wrote:
> > > @@ -561,6 +568,6 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned
> > > int speed)
> > > */
> > > int __nfp_eth_set_split(struct nfp_nsp *nsp, unsigned int lanes)
> > > {
> > > - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
> > > + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
> > > lanes, NSP_ETH_CTRL_SET_LANES);
> > > }
> >
> > Do you plan to send this as an official patch?
>
> Sorry... Dirk is prepping a larger series for this area of code and it
> got stuck there a bit :S
No prob, just wanted to make sure it didn't slip under the radar.
> He says it's coming any day now...
Great, thanks!
Re: [PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
Hi Jakub,
El Wed, Oct 04, 2017 at 11:17:24AM -0700 Jakub Kicinski ha dit:
> On Wed, 4 Oct 2017 10:42:42 -0700, Manoj Gupta wrote:
> > Hi Jakub,
> >
> > I had discussed about supporting this code with some clang developers.
> > However, the consensus was this code relies on a specific GCC optimizer
> > behavior and Clang does not share the same behavior by design.
>
> Hm. I find surprising that constant propagation to inlined functions
> is considered something GCC-specific rather than obvious/basic.
>
> > Note that even GCC can't compile this code at -O0.
>
> Yes, that makes me feel uncomfortable... Could you try this?
>
> ---8<---
>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> index f6f7c085f8e0..47251396fcae 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> @@ -469,10 +469,10 @@ int nfp_eth_set_configured(struct nfp_cpp *cpp,
> unsigned int idx, bool configed)
> return nfp_eth_config_commit_end(nsp);
> }
>
> -/* Force inline, FIELD_* macroes require masks to be compilation-time known
> */
> -static __always_inline int
> +static int
> nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
> -const u64 mask, unsigned int val, const u64 ctrl_bit)
> +const u64 mask, const unsigned int shift,
> +unsigned int val, const u64 ctrl_bit)
> {
> union eth_table_entry *entries = nfp_nsp_config_entries(nsp);
> unsigned int idx = nfp_nsp_config_idx(nsp);
> @@ -489,11 +489,11 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned
> int raw_idx,
>
> /* Check if we are already in requested state */
> reg = le64_to_cpu(entries[idx].raw[raw_idx]);
> - if (val == FIELD_GET(mask, reg))
> + if (val == (reg & mask) >> shift)
> return 0;
>
> reg &= ~mask;
> - reg |= FIELD_PREP(mask, val);
> + reg |= (val << shift) & mask;
> entries[idx].raw[raw_idx] = cpu_to_le64(reg);
>
> entries[idx].control |= cpu_to_le64(ctrl_bit);
> @@ -503,6 +503,13 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int
> raw_idx,
> return 0;
> }
>
> +#define NFP_ETH_SET_BIT_CONFIG(nsp, raw_idx, mask, val, ctrl_bit)\
> + ({ \
> + __BF_FIELD_CHECK(mask, 0ULL, val, "NFP_ETH_SET_BIT_CONFIG: "); \
> + nfp_eth_set_bit_config(nsp, raw_idx, mask, __bf_shf(mask), \
> +val, ctrl_bit); \
> + })
> +
> /**
> * __nfp_eth_set_aneg() - set PHY autonegotiation control bit
> * @nsp: NFP NSP handle returned from nfp_eth_config_start()
> @@ -515,7 +522,7 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int
> raw_idx,
> */
> int __nfp_eth_set_aneg(struct nfp_nsp *nsp, enum nfp_eth_aneg mode)
> {
> - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE,
> + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE,
> NSP_ETH_STATE_ANEG, mode,
> NSP_ETH_CTRL_SET_ANEG);
> }
> @@ -544,7 +551,7 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int
> speed)
> return -EINVAL;
> }
>
> - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE,
> + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE,
> NSP_ETH_STATE_RATE, rate,
> NSP_ETH_CTRL_SET_RATE);
> }
> @@ -561,6 +568,6 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int
> speed)
> */
> int __nfp_eth_set_split(struct nfp_nsp *nsp, unsigned int lanes)
> {
> - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
> + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
> lanes, NSP_ETH_CTRL_SET_LANES);
> }
Do you plan to send this as an official patch?
Thanks
Matthias
[PATCH] wimax/i2400m: Remove VLAIS
From: Behan Webster <beh...@converseincode.com>
Convert Variable Length Array in Struct (VLAIS) to valid C by converting
local struct definition to use a flexible array. The structure is only
used to define a cast of a buffer so the size of the struct is not used
to allocate storage.
Signed-off-by: Behan Webster <beh...@converseincode.com>
Signed-off-by: Mark Charebois <charl...@gmail.com>
Suggested-by: Arnd Bergmann <a...@arndb.de>
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
drivers/net/wimax/i2400m/fw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index c9c711dcd0e6..a89b5685e68b 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -652,7 +652,7 @@ static int i2400m_download_chunk(struct i2400m *i2400m,
const void *chunk,
struct device *dev = i2400m_dev(i2400m);
struct {
struct i2400m_bootrom_header cmd;
- u8 cmd_payload[chunk_len];
+ u8 cmd_payload[];
} __packed *buf;
struct i2400m_bootrom_header ack;
--
2.14.2.920.gcf0c67979c-goog
Re: [PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
El Wed, Oct 04, 2017 at 07:13:26PM -0700 Manoj Gupta ha dit: > On Wed, Oct 4, 2017 at 7:06 PM, Jakub Kicinski > <jakub.kicin...@netronome.com> wrote: > > On Wed, 4 Oct 2017 18:50:04 -0700, Manoj Gupta wrote: > >> On Wed, Oct 4, 2017 at 5:56 PM, Jakub Kicinski wrote: > >> > On Wed, 4 Oct 2017 17:38:22 -0700, Manoj Gupta wrote: > >> >> On Wed, Oct 4, 2017 at 4:25 PM, Jakub Kicinski wrote: > >> >> > On Wed, 4 Oct 2017 16:16:49 -0700, Matthias Kaehlcke wrote: > >> >> >> > > Thanks for the suggestion. This seems a viable alternative if > >> >> >> > > David > >> >> >> > > and the NFP owners can live without the extra checking provided > >> >> >> > > by > >> >> >> > > __BF_FIELD_CHECK. > >> >> >> > > >> >> >> > The reason the __BF_FIELD_CHECK refuses to compile non-constant > >> >> >> > masks > >> >> >> > is that it will require runtime ffs on the mask, which is > >> >> >> > potentially > >> >> >> > costly. I would also feel quite stupid adding those macros to the > >> >> >> > nfp > >> >> >> > driver, given that I specifically created the bitfield.h header to > >> >> >> > not > >> >> >> > have to reimplement these in every driver I write/maintain. > >> >> >> > >> >> >> That make sense, thanks for providing more context. > >> >> >> > >> >> >> > Can you please test the patch I provided in the other reply? > >> >> >> > >> >> >> With this patch there are no errors when building the kernel with > >> >> >> clang. > >> >> > > >> >> > Cool, thanks for checking! I will run it through full tests and queue > >> >> > for upstreaming :) > >> >> > >> >> Just to let you know, using __BF_FIELD_CHECK macro will not Link with > >> >> -O0 (GCC or Clang) since references to __compiletime_assert_xxx will > >> >> not be cleaned up. > >> > > >> > Do you mean the current nfp_eth_set_bit_config() will not work with -O0 > >> > on either complier, or any use of __BF_FIELD_CHECK() will not compile > >> > with -O0? > >> > >> Any use of __BF_FIELD_CHECK. The code will compile but not link since > >> calls to compiletime_assert_xxx (added by compiletime_assert > >> macro) will not be removed in -O0. > > > > Why would that be, it's just a macro? Does it by extension mean any > > use of BUILD_BUG_ON_MSG() will not compile with -O0? > > You have to look at the the code added once the macro is expanded :). > Please look at implementation of compiletime_assert at > http://elixir.free-electrons.com/linux/v4.12.14/source/include/linux/compiler.h#L507 > It creates a call to __compiler_assert_xxx inside a loop which is not > cleaned up in -O0. I just saw that v4.14 will have a fix for that: commit c03567a8e8d5cf2aaca40e605c48f319dc2ead57 Author: Joe Stringer <j...@ovn.org> Date: Thu Aug 31 16:15:33 2017 -0700 include/linux/compiler.h: don't perform compiletime_assert with -O0 Obviously this means that the checks aren't performed, however that shouldn't be an issue since AFAIK the kernel doesn't officially support -O0 builds in the first place.
Re: [PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
Hi Jakub,
El Wed, Oct 04, 2017 at 03:22:03PM -0700 Jakub Kicinski ha dit:
> On Wed, 4 Oct 2017 11:49:57 -0700, Matthias Kaehlcke wrote:
> > Hi Joe,
> >
> > El Wed, Oct 04, 2017 at 11:07:19AM -0700 Joe Perches ha dit:
> >
> > > On Tue, 2017-10-03 at 13:05 -0700, Matthias Kaehlcke wrote:
> > > > nfp_eth_set_bit_config() is marked as __always_inline to allow gcc to
> > > > identify the 'mask' parameter as known to be constant at compile time,
> > > > which is required to use the FIELD_GET() macro.
> > > >
> > > > The forced inlining does the trick for gcc, but for kernel builds with
> > > > clang it results in undefined symbols:
> > >
> > > Can't you use local different FIELD_PREP/FIELD_GET macros
> > > with a different name without the BUILD_BUG tests?
> > >
> > > i.e.:
> > >
> > > #define NFP_FIELD_PREP(_mask, _val) \
> > > ({\
> > > ((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask); \
> > > })
> > >
> > > #define NFP_FIELD_GET(_mask, _reg)\
> > > ({\
> > > (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
> > > })
> > >
> > > Then the __always_inline can be removed from
> > > nfp_eth_set_bit_config too.
> >
> > Thanks for the suggestion. This seems a viable alternative if David
> > and the NFP owners can live without the extra checking provided by
> > __BF_FIELD_CHECK.
>
> The reason the __BF_FIELD_CHECK refuses to compile non-constant masks
> is that it will require runtime ffs on the mask, which is potentially
> costly. I would also feel quite stupid adding those macros to the nfp
> driver, given that I specifically created the bitfield.h header to not
> have to reimplement these in every driver I write/maintain.
That make sense, thanks for providing more context.
> Can you please test the patch I provided in the other reply?
With this patch there are no errors when building the kernel with
clang.
Thanks!
Matthias
Re: [PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
Hi Joe,
El Wed, Oct 04, 2017 at 11:07:19AM -0700 Joe Perches ha dit:
> On Tue, 2017-10-03 at 13:05 -0700, Matthias Kaehlcke wrote:
> > nfp_eth_set_bit_config() is marked as __always_inline to allow gcc to
> > identify the 'mask' parameter as known to be constant at compile time,
> > which is required to use the FIELD_GET() macro.
> >
> > The forced inlining does the trick for gcc, but for kernel builds with
> > clang it results in undefined symbols:
>
> Can't you use local different FIELD_PREP/FIELD_GET macros
> with a different name without the BUILD_BUG tests?
>
> i.e.:
>
> #define NFP_FIELD_PREP(_mask, _val) \
> ({\
> ((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask); \
> })
>
> #define NFP_FIELD_GET(_mask, _reg)\
> ({\
> (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
> })
>
> Then the __always_inline can be removed from
> nfp_eth_set_bit_config too.
Thanks for the suggestion. This seems a viable alternative if David
and the NFP owners can live without the extra checking provided by
__BF_FIELD_CHECK.
Re: [PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
El Wed, Oct 04, 2017 at 11:17:24AM -0700 Jakub Kicinski ha dit:
> On Wed, 4 Oct 2017 10:42:42 -0700, Manoj Gupta wrote:
> > Hi Jakub,
> >
> > I had discussed about supporting this code with some clang developers.
> > However, the consensus was this code relies on a specific GCC optimizer
> > behavior and Clang does not share the same behavior by design.
>
> Hm. I find surprising that constant propagation to inlined functions
> is considered something GCC-specific rather than obvious/basic.
>
> > Note that even GCC can't compile this code at -O0.
>
> Yes, that makes me feel uncomfortable... Could you try this?
The kernel as a whole does not build with -O0, so I couldn't try
that. I know Manoj (who isn't a kernel dev) isolated the
compiletime_assert macros and encountered the same 'undefined
reference' errors with gcc -O0 that we are seeing with clang.
This is due to:
"if you use it in an inlined function and pass an argument of the
function as the argument to the built-in, GCC never returns 1 when
you call the inline function with a string constant or compound
literal (see Compound Literals) and does not return 1 when you pass a
constant numeric value to the inline function unless you specify the
-O option."
https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Other-Builtins.html
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> index f6f7c085f8e0..47251396fcae 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> @@ -469,10 +469,10 @@ int nfp_eth_set_configured(struct nfp_cpp *cpp,
> unsigned int idx, bool configed)
> return nfp_eth_config_commit_end(nsp);
> }
>
> -/* Force inline, FIELD_* macroes require masks to be compilation-time known
> */
> -static __always_inline int
> +static int
> nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
> -const u64 mask, unsigned int val, const u64 ctrl_bit)
> +const u64 mask, const unsigned int shift,
> +unsigned int val, const u64 ctrl_bit)
> {
> union eth_table_entry *entries = nfp_nsp_config_entries(nsp);
> unsigned int idx = nfp_nsp_config_idx(nsp);
> @@ -489,11 +489,11 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned
> int raw_idx,
>
> /* Check if we are already in requested state */
> reg = le64_to_cpu(entries[idx].raw[raw_idx]);
> - if (val == FIELD_GET(mask, reg))
> + if (val == (reg & mask) >> shift)
> return 0;
>
> reg &= ~mask;
> - reg |= FIELD_PREP(mask, val);
> + reg |= (val << shift) & mask;
> entries[idx].raw[raw_idx] = cpu_to_le64(reg);
>
> entries[idx].control |= cpu_to_le64(ctrl_bit);
> @@ -503,6 +503,13 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int
> raw_idx,
> return 0;
> }
>
> +#define NFP_ETH_SET_BIT_CONFIG(nsp, raw_idx, mask, val, ctrl_bit)\
> + ({ \
> + __BF_FIELD_CHECK(mask, 0ULL, val, "NFP_ETH_SET_BIT_CONFIG: "); \
> + nfp_eth_set_bit_config(nsp, raw_idx, mask, __bf_shf(mask), \
> +val, ctrl_bit); \
> + })
> +
> /**
> * __nfp_eth_set_aneg() - set PHY autonegotiation control bit
> * @nsp: NFP NSP handle returned from nfp_eth_config_start()
> @@ -515,7 +522,7 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int
> raw_idx,
> */
> int __nfp_eth_set_aneg(struct nfp_nsp *nsp, enum nfp_eth_aneg mode)
> {
> - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE,
> + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE,
> NSP_ETH_STATE_ANEG, mode,
> NSP_ETH_CTRL_SET_ANEG);
> }
> @@ -544,7 +551,7 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int
> speed)
> return -EINVAL;
> }
>
> - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE,
> + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE,
> NSP_ETH_STATE_RATE, rate,
> NSP_ETH_CTRL_SET_RATE);
> }
> @@ -561,6 +568,6 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int
> speed)
> */
> int __nfp_eth_set_split(struct nfp_nsp *nsp, unsigned int lanes)
> {
> - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
> + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
> lanes, NSP_ETH_CTRL_SET_LANES);
> }
Re: [PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
El Tue, Oct 03, 2017 at 02:50:00PM -0700 Jakub Kicinski ha dit: > On Tue, 3 Oct 2017 13:05:46 -0700, Matthias Kaehlcke wrote: > > nfp_eth_set_bit_config() is marked as __always_inline to allow gcc to > > identify the 'mask' parameter as known to be constant at compile time, > > which is required to use the FIELD_GET() macro. > > > > The forced inlining does the trick for gcc, but for kernel builds with > > clang it results in undefined symbols: > > > > drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.o: In function > > `__nfp_eth_set_aneg': > > drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c:(.text+0x787): > > undefined reference to `__compiletime_assert_492' > > drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c:(.text+0x7b1): > > undefined reference to `__compiletime_assert_496' > > > > These __compiletime_assert_xyx() calls would have been optimized away if > > the compiler had seen 'mask' as a constant. > > > > Convert nfp_eth_set_bit_config() into a macro, which allows both gcc and > > clang to identify 'mask' as a compile time constant. > > > > Signed-off-by: Matthias Kaehlcke <m...@chromium.org> > > :( > > Is there no chance of fixing the constant propagation in the compiler? LLVM developers are reluctant and would like us kernel folks to evaluate possible alternatives for the affected code first: https://bugs.llvm.org/show_bug.cgi?id=4898 Given that this doesn't seem to be a widespread issue in the kernel personally I would consider the conversion to a macro in this case an acceptable solution, though it is definitely ugly. However I'm not the owner of the driver or the subsystem, so my opinion doesn't really carry much weight here ;-) Any ideas about other, less ugly alternatives? Matthias
[PATCH] nfp: convert nfp_eth_set_bit_config() into a macro
nfp_eth_set_bit_config() is marked as __always_inline to allow gcc to
identify the 'mask' parameter as known to be constant at compile time,
which is required to use the FIELD_GET() macro.
The forced inlining does the trick for gcc, but for kernel builds with
clang it results in undefined symbols:
drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.o: In function
`__nfp_eth_set_aneg':
drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c:(.text+0x787):
undefined reference to `__compiletime_assert_492'
drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c:(.text+0x7b1):
undefined reference to `__compiletime_assert_496'
These __compiletime_assert_xyx() calls would have been optimized away if
the compiler had seen 'mask' as a constant.
Convert nfp_eth_set_bit_config() into a macro, which allows both gcc and
clang to identify 'mask' as a compile time constant.
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
I am aware that a lengthy macro is not a pretty solution, I'm open for
better suggestions.
Note: The patch has been build-tested only since I don't have any NFP
hardware.
.../ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c | 67 +++---
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
index f6f7c085f8e0..e9c635867918 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
@@ -469,39 +469,40 @@ int nfp_eth_set_configured(struct nfp_cpp *cpp, unsigned
int idx, bool configed)
return nfp_eth_config_commit_end(nsp);
}
-/* Force inline, FIELD_* macroes require masks to be compilation-time known */
-static __always_inline int
-nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
- const u64 mask, unsigned int val, const u64 ctrl_bit)
-{
- union eth_table_entry *entries = nfp_nsp_config_entries(nsp);
- unsigned int idx = nfp_nsp_config_idx(nsp);
- u64 reg;
-
- /* Note: set features were added in ABI 0.14 but the error
-* codes were initially not populated correctly.
-*/
- if (nfp_nsp_get_abi_ver_minor(nsp) < 17) {
- nfp_err(nfp_nsp_cpp(nsp),
- "set operations not supported, please update flash\n");
- return -EOPNOTSUPP;
- }
-
- /* Check if we are already in requested state */
- reg = le64_to_cpu(entries[idx].raw[raw_idx]);
- if (val == FIELD_GET(mask, reg))
- return 0;
-
- reg &= ~mask;
- reg |= FIELD_PREP(mask, val);
- entries[idx].raw[raw_idx] = cpu_to_le64(reg);
-
- entries[idx].control |= cpu_to_le64(ctrl_bit);
-
- nfp_nsp_config_set_modified(nsp, true);
-
- return 0;
-}
+#define nfp_eth_set_bit_config(nsp, raw_idx, mask, val, ctrl_bit) \
+({ \
+ union eth_table_entry *entries = nfp_nsp_config_entries(nsp); \
+ unsigned int idx = nfp_nsp_config_idx(nsp); \
+ u64 reg;\
+ int rc; \
+ \
+ /* Note: set features were added in ABI 0.14 but the error */ \
+ /* codes were initially not populated correctly. */ \
+ if (nfp_nsp_get_abi_ver_minor(nsp) < 17) { \
+ nfp_err(nfp_nsp_cpp(nsp), \
+ "set operations not supported, please update flash\n");
\
+ rc = -EOPNOTSUPP; \
+ goto out; \
+ } \
+ \
+ rc = 0; \
+ \
+ /* Check if we are already in requested state */\
+ reg = le64_to_cpu(entries[idx].raw[raw_idx]); \
+ if (val == FIELD_GET(mask, reg))\
+ goto out; \
+ \
+ reg &= ~mask; \
+ reg |= FIELD_PREP(mask, val); \
+ entries[idx].raw[raw_idx] = cpu_to_le64(reg); \
+ \
+ entr
[PATCH] netpoll: Fix device name check in netpoll_setup()
Apparently netpoll_setup() assumes that netpoll.dev_name is a pointer
when checking if the device name is set:
if (np->dev_name) {
...
However the field is a character array, therefore the condition always
yields true. Check instead whether the first byte of the array has a
non-zero value.
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
net/core/netpoll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 8357f164c660..912731bed7b7 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -666,7 +666,7 @@ int netpoll_setup(struct netpoll *np)
int err;
rtnl_lock();
- if (np->dev_name) {
+ if (np->dev_name[0]) {
struct net *net = current->nsproxy->net_ns;
ndev = __dev_get_by_name(net, np->dev_name);
}
--
2.14.0.rc0.284.gd933b75aa4-goog
[PATCH] net: jme: Remove unused functions
The functions jme_restart_tx_engine(), jme_pause_rx() and
jme_resume_rx() are not used. Removing them fixes the following warnings
when building with clang:
drivers/net/ethernet/jme.c:694:1: error: unused function
'jme_restart_tx_engine' [-Werror,-Wunused-function]
drivers/net/ethernet/jme.c:2393:20: error: unused function
'jme_pause_rx' [-Werror,-Wunused-function]
drivers/net/ethernet/jme.c:2406:20: error: unused function
'jme_resume_rx' [-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
drivers/net/ethernet/jme.c | 42 --
1 file changed, 42 deletions(-)
diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
index f580b49e6b67..0e5083a48937 100644
--- a/drivers/net/ethernet/jme.c
+++ b/drivers/net/ethernet/jme.c
@@ -691,17 +691,6 @@ jme_enable_tx_engine(struct jme_adapter *jme)
}
static inline void
-jme_restart_tx_engine(struct jme_adapter *jme)
-{
- /*
-* Restart TX Engine
-*/
- jwrite32(jme, JME_TXCS, jme->reg_txcs |
- TXCS_SELECT_QUEUE0 |
- TXCS_ENABLE);
-}
-
-static inline void
jme_disable_tx_engine(struct jme_adapter *jme)
{
int i;
@@ -2382,37 +2371,6 @@ jme_tx_timeout(struct net_device *netdev)
jme_reset_link(jme);
}
-static inline void jme_pause_rx(struct jme_adapter *jme)
-{
- atomic_dec(>link_changing);
-
- jme_set_rx_pcc(jme, PCC_OFF);
- if (test_bit(JME_FLAG_POLL, >flags)) {
- JME_NAPI_DISABLE(jme);
- } else {
- tasklet_disable(>rxclean_task);
- tasklet_disable(>rxempty_task);
- }
-}
-
-static inline void jme_resume_rx(struct jme_adapter *jme)
-{
- struct dynpcc_info *dpi = &(jme->dpi);
-
- if (test_bit(JME_FLAG_POLL, >flags)) {
- JME_NAPI_ENABLE(jme);
- } else {
- tasklet_enable(>rxclean_task);
- tasklet_enable(>rxempty_task);
- }
- dpi->cur= PCC_P1;
- dpi->attempt= PCC_P1;
- dpi->cnt= 0;
- jme_set_rx_pcc(jme, PCC_P1);
-
- atomic_inc(>link_changing);
-}
-
static void
jme_get_drvinfo(struct net_device *netdev,
struct ethtool_drvinfo *info)
--
2.13.0.219.gdb65acc882-goog
[PATCH] net1080: Remove unused function nc_dump_ttl()
The function is not used, removing it fixes the following warning when
building with clang:
drivers/net/usb/net1080.c:271:20: error: unused function
'nc_dump_ttl' [-Werror,-Wunused-function]
Also remove the definition of TTL_THIS, which is only used in
nc_dump_ttl()
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
drivers/net/usb/net1080.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index 4cbdb1307f3e..3202c19df83d 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -264,17 +264,9 @@ static inline void nc_dump_status(struct usbnet *dev, u16
status)
* TTL register
*/
-#defineTTL_THIS(ttl) (0x00ff & ttl)
#defineTTL_OTHER(ttl) (0x00ff & (ttl >> 8))
#define MK_TTL(this,other) ((u16)(((other)<<8)|(0x00ff&(this
-static inline void nc_dump_ttl(struct usbnet *dev, u16 ttl)
-{
- netif_dbg(dev, link, dev->net, "net1080 %s-%s ttl 0x%x this = %d, other
= %d\n",
- dev->udev->bus->bus_name, dev->udev->devpath,
- ttl, TTL_THIS(ttl), TTL_OTHER(ttl));
-}
-
/*-*/
static int net1080_reset(struct usbnet *dev)
@@ -308,7 +300,6 @@ static int net1080_reset(struct usbnet *dev)
goto done;
}
ttl = vp;
- // nc_dump_ttl(dev, ttl);
nc_register_write(dev, REG_TTL,
MK_TTL(NC_READ_TTL_MS, TTL_OTHER(ttl)) );
--
2.13.0.303.g4ebf302169-goog
[PATCH] r8152: Remove unused function usb_ocp_read()
The function is not used, removing it fixes the following warning when
building with clang:
drivers/net/usb/r8152.c:825:5: error: unused function 'usb_ocp_read'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
drivers/net/usb/r8152.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ddc62cb69be8..e902df9595b9 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -841,12 +841,6 @@ int pla_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
u16 size, void *data)
}
static inline
-int usb_ocp_read(struct r8152 *tp, u16 index, u16 size, void *data)
-{
- return generic_ocp_read(tp, index, size, data, MCU_TYPE_USB);
-}
-
-static inline
int usb_ocp_write(struct r8152 *tp, u16 index, u16 byteen, u16 size, void
*data)
{
return generic_ocp_write(tp, index, byteen, size, data, MCU_TYPE_USB);
--
2.13.0.303.g4ebf302169-goog
Re: [PATCH] net1080: Mark nc_dump_ttl() as __maybe_unused
Hi David, El Thu, May 18, 2017 at 10:48:08AM -0400 David Miller ha dit: > From: Matthias Kaehlcke <m...@chromium.org> > Date: Wed, 17 May 2017 15:17:08 -0700 > > > The function is not used, but it looks useful for debugging. Adding the > > attribute fixes the following clang warning: > > > > drivers/net/usb/net1080.c:271:20: error: unused function > > 'nc_dump_ttl' [-Werror,-Wunused-function] > > > > Signed-off-by: Matthias Kaehlcke <m...@chromium.org> > > For this and the r8152 patch, I definitely prefer that the function is > removed. > > If someone needs them, they can pull it out of the GIT history. Thanks for you comments, I'll send out updated patches soon.
[PATCH] r8152: Mark usb_ocp_read() as __maybe_unused
The function is not used, but is probably kept around for debugging and
symmetry with usb_ocp_write(). Adding the attribute fixes the following
clang warning:
drivers/net/usb/r8152.c:825:5: error: unused function 'usb_ocp_read'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
drivers/net/usb/r8152.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ddc62cb69be8..12776230ab96 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -840,7 +840,7 @@ int pla_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
u16 size, void *data)
return generic_ocp_write(tp, index, byteen, size, data, MCU_TYPE_PLA);
}
-static inline
+static inline __maybe_unused
int usb_ocp_read(struct r8152 *tp, u16 index, u16 size, void *data)
{
return generic_ocp_read(tp, index, size, data, MCU_TYPE_USB);
--
2.13.0.303.g4ebf302169-goog
[PATCH] net1080: Mark nc_dump_ttl() as __maybe_unused
The function is not used, but it looks useful for debugging. Adding the
attribute fixes the following clang warning:
drivers/net/usb/net1080.c:271:20: error: unused function
'nc_dump_ttl' [-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
drivers/net/usb/net1080.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index 4cbdb1307f3e..7ade2119f462 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -268,7 +268,7 @@ static inline void nc_dump_status(struct usbnet *dev, u16
status)
#defineTTL_OTHER(ttl) (0x00ff & (ttl >> 8))
#define MK_TTL(this,other) ((u16)(((other)<<8)|(0x00ff&(this
-static inline void nc_dump_ttl(struct usbnet *dev, u16 ttl)
+static inline void __maybe_unused nc_dump_ttl(struct usbnet *dev, u16 ttl)
{
netif_dbg(dev, link, dev->net, "net1080 %s-%s ttl 0x%x this = %d, other
= %d\n",
dev->udev->bus->bus_name, dev->udev->devpath,
--
2.13.0.303.g4ebf302169-goog
Re: [PATCH] netfilter: ctnetlink: Make some parameters integer to avoid enum mismatch
El Wed, Apr 19, 2017 at 11:39:20AM -0700 Matthias Kaehlcke ha dit: > Not all parameters passed to ctnetlink_parse_tuple() and > ctnetlink_exp_dump_tuple() match the enum type in the signatures of these > functions. Since this is intended change the argument type of to be an int > value. > > Signed-off-by: Matthias Kaehlcke <m...@chromium.org> > --- Ping, any comments on this patch? Thanks Matthias
Re: [PATCH] netfilter: ctnetlink: Make some parameters integer to avoid enum mismatch
El Wed, Apr 19, 2017 at 12:41:10PM -0700 Joe Perches ha dit:
> On Wed, 2017-04-19 at 11:39 -0700, Matthias Kaehlcke wrote:
> > Not all parameters passed to ctnetlink_parse_tuple() and
> > ctnetlink_exp_dump_tuple() match the enum type in the signatures of these
> > functions.
>
> Maybe that should be changed/fixed.
Please see the previous discussion at
http://www.spinics.net/lists/netfilter-devel/msg47540.html
> > Since this is intended change the argument type of to be an int
> > value.
>
> u32 is not int, it's unsigned int
I would argue that an unsigned int is an int(eger) and considered it
an unnecessary detail for the commit message to be explicit. I can
change it if others deem it incorrect.
Cheers
Matthias
> > Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
> > ---
> > net/netfilter/nf_conntrack_netlink.c | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_netlink.c
> > b/net/netfilter/nf_conntrack_netlink.c
> > index dc7dfd68fafe..775eb5d9165b 100644
> > --- a/net/netfilter/nf_conntrack_netlink.c
> > +++ b/net/netfilter/nf_conntrack_netlink.c
> > @@ -1006,9 +1006,8 @@ static const struct nla_policy
> > tuple_nla_policy[CTA_TUPLE_MAX+1] = {
> >
> > static int
> > ctnetlink_parse_tuple(const struct nlattr * const cda[],
> > - struct nf_conntrack_tuple *tuple,
> > - enum ctattr_type type, u_int8_t l3num,
> > - struct nf_conntrack_zone *zone)
> > + struct nf_conntrack_tuple *tuple, u32 type,
> > + u_int8_t l3num, struct nf_conntrack_zone *zone)
> > {
> > struct nlattr *tb[CTA_TUPLE_MAX+1];
> > int err;
> > @@ -2443,7 +2442,7 @@ static struct nfnl_ct_hook ctnetlink_glue_hook = {
> >
> > static int ctnetlink_exp_dump_tuple(struct sk_buff *skb,
> > const struct nf_conntrack_tuple *tuple,
> > - enum ctattr_expect type)
> > + u32 type)
> > {
> > struct nlattr *nest_parms;
> >
[PATCH] netfilter: ctnetlink: Make some parameters integer to avoid enum mismatch
Not all parameters passed to ctnetlink_parse_tuple() and
ctnetlink_exp_dump_tuple() match the enum type in the signatures of these
functions. Since this is intended change the argument type of to be an int
value.
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
net/netfilter/nf_conntrack_netlink.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c
b/net/netfilter/nf_conntrack_netlink.c
index dc7dfd68fafe..775eb5d9165b 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1006,9 +1006,8 @@ static const struct nla_policy
tuple_nla_policy[CTA_TUPLE_MAX+1] = {
static int
ctnetlink_parse_tuple(const struct nlattr * const cda[],
- struct nf_conntrack_tuple *tuple,
- enum ctattr_type type, u_int8_t l3num,
- struct nf_conntrack_zone *zone)
+ struct nf_conntrack_tuple *tuple, u32 type,
+ u_int8_t l3num, struct nf_conntrack_zone *zone)
{
struct nlattr *tb[CTA_TUPLE_MAX+1];
int err;
@@ -2443,7 +2442,7 @@ static struct nfnl_ct_hook ctnetlink_glue_hook = {
static int ctnetlink_exp_dump_tuple(struct sk_buff *skb,
const struct nf_conntrack_tuple *tuple,
- enum ctattr_expect type)
+ u32 type)
{
struct nlattr *nest_parms;
--
2.12.2.816.g281164-goog
[PATCH] nl80211: Fix enum type of variable in nl80211_put_sta_rate()
rate_flg is of type 'enum nl80211_attrs', however it is assigned with 'enum nl80211_rate_info' values. Change the type of rate_flg accordingly. Signed-off-by: Matthias Kaehlcke <m...@chromium.org> --- net/wireless/nl80211.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 2312dc2ffdb9..9af21a21ea6b 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -4151,7 +4151,7 @@ static bool nl80211_put_sta_rate(struct sk_buff *msg, struct rate_info *info, struct nlattr *rate; u32 bitrate; u16 bitrate_compat; - enum nl80211_attrs rate_flg; + enum nl80211_rate_info rate_flg; rate = nla_nest_start(msg, attr); if (!rate) -- 2.12.2.762.g0e3151a226-goog
[PATCH] mac80211: ibss: Fix channel type enum in ieee80211_sta_join_ibss()
cfg80211_chandef_create() expects an 'enum nl80211_channel_type' as channel type however in ieee80211_sta_join_ibss() NL80211_CHAN_WIDTH_20_NOHT is passed in two occasions, which is of the enum type 'nl80211_chan_width'. Change the value to NL80211_CHAN_NO_HT (20 MHz, non-HT channel) of the channel type enum. Signed-off-by: Matthias Kaehlcke <m...@chromium.org> --- net/mac80211/ibss.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c index 98999d3d5262..e957351976a2 100644 --- a/net/mac80211/ibss.c +++ b/net/mac80211/ibss.c @@ -425,7 +425,7 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata, case NL80211_CHAN_WIDTH_5: case NL80211_CHAN_WIDTH_10: cfg80211_chandef_create(, cbss->channel, - NL80211_CHAN_WIDTH_20_NOHT); + NL80211_CHAN_NO_HT); chandef.width = sdata->u.ibss.chandef.width; break; case NL80211_CHAN_WIDTH_80: @@ -437,7 +437,7 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata, default: /* fall back to 20 MHz for unsupported modes */ cfg80211_chandef_create(, cbss->channel, - NL80211_CHAN_WIDTH_20_NOHT); + NL80211_CHAN_NO_HT); break; } -- 2.12.2.762.g0e3151a226-goog
[PATCH v2] cfg80211: Fix array-bounds warning in fragment copy
__ieee80211_amsdu_copy_frag intentionally initializes a pointer to
array[-1] to increment it later to valid values. clang rightfully
generates an array-bounds warning on the initialization statement.
Initialize the pointer to array[0] and change the algorithm from
increment before to increment after consume.
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
Note: Resent to include linux-wireless in cc
net/wireless/util.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 68e5f2ecee1a..52795ae5337f 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -659,7 +659,7 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
int offset, int len)
{
struct skb_shared_info *sh = skb_shinfo(skb);
- const skb_frag_t *frag = >frags[-1];
+ const skb_frag_t *frag = >frags[0];
struct page *frag_page;
void *frag_ptr;
int frag_len, frag_size;
@@ -672,10 +672,10 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
while (offset >= frag_size) {
offset -= frag_size;
- frag++;
frag_page = skb_frag_page(frag);
frag_ptr = skb_frag_address(frag);
frag_size = skb_frag_size(frag);
+ frag++;
}
frag_ptr += offset;
@@ -687,12 +687,12 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
len -= cur_len;
while (len > 0) {
- frag++;
frag_len = skb_frag_size(frag);
cur_len = min(len, frag_len);
__frame_add_frag(frame, skb_frag_page(frag),
skb_frag_address(frag), cur_len, frag_len);
len -= cur_len;
+ frag++;
}
}
--
2.12.2.715.g7642488e1d-goog
Re: [PATCH v2] cfg80211: Fix array-bounds warning in fragment copy
El Mon, Mar 27, 2017 at 12:58:22PM -0700 Matthias Kaehlcke ha dit:
> __ieee80211_amsdu_copy_frag intentionally initializes a pointer to
> array[-1] to increment it later to valid values. clang rightfully
> generates an array-bounds warning on the initialization statement.
>
> Initialize the pointer to array[0] and change the algorithm from
> increment before to increment after consume.
>
> Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
> ---
> net/wireless/util.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 68e5f2ecee1a..52795ae5337f 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -659,7 +659,7 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
> sk_buff *frame,
> int offset, int len)
> {
> struct skb_shared_info *sh = skb_shinfo(skb);
> - const skb_frag_t *frag = >frags[-1];
> + const skb_frag_t *frag = >frags[0];
> struct page *frag_page;
> void *frag_ptr;
> int frag_len, frag_size;
> @@ -672,10 +672,10 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
> sk_buff *frame,
>
> while (offset >= frag_size) {
> offset -= frag_size;
> - frag++;
> frag_page = skb_frag_page(frag);
> frag_ptr = skb_frag_address(frag);
> frag_size = skb_frag_size(frag);
> + frag++;
> }
>
> frag_ptr += offset;
> @@ -687,12 +687,12 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
> sk_buff *frame,
> len -= cur_len;
>
> while (len > 0) {
> - frag++;
> frag_len = skb_frag_size(frag);
> cur_len = min(len, frag_len);
> __frame_add_frag(frame, skb_frag_page(frag),
>skb_frag_address(frag), cur_len, frag_len);
> len -= cur_len;
> + frag++;
> }
> }
>
Ping, any feedback on this patch?
Thanks
Matthias
Re: [PATCH] ath9k: Add cast to u8 to FREQ2FBIN macro
Hi Joe,
El Thu, Apr 06, 2017 at 02:29:20PM -0700 Joe Perches ha dit:
> On Thu, 2017-04-06 at 14:21 -0700, Matthias Kaehlcke wrote:
> > The macro results are assigned to u8 variables/fields. Adding the cast
> > fixes plenty of clang warnings about "implicit conversion from 'int' to
> > 'u8'".
> >
> > Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
> > ---
> > drivers/net/wireless/ath/ath9k/eeprom.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h
> > b/drivers/net/wireless/ath/ath9k/eeprom.h
> > index 30bf722e33ed..31390af6c33e 100644
> > --- a/drivers/net/wireless/ath/ath9k/eeprom.h
> > +++ b/drivers/net/wireless/ath/ath9k/eeprom.h
> > @@ -106,7 +106,7 @@
> > #define AR9285_RDEXT_DEFAULT0x1F
> >
> > #define ATH9K_POW_SM(_r, _s) (((_r) & 0x3f) << (_s))
> > -#define FREQ2FBIN(x, y)((y) ? ((x) - 2300) : (((x) - 4800) /
> > 5))
> > +#define FREQ2FBIN(x, y)(u8)((y) ? ((x) - 2300) : (((x) - 4800)
> > / 5))
>
> Maybe better to use:
>
> static inline u8 FREQ2FBIN(int x, int y)
> {
> if (y)
> return x - 2300;
> return (x - 4800) / 5;
> }
Thanks for your suggestion! Unfortunately in this case an inline
function is not suitable since FREQ2FBIN() is mostly used for
structure initialization:
static const struct ar9300_eeprom ar9300_default = {
...
.calFreqPier2G = {
FREQ2FBIN(2412, 1),
FREQ2FBIN(2437, 1),
FREQ2FBIN(2472, 1),
},
...
Cheers
Matthias
[PATCH v2] mac80211: Fix clang warning about constant operand in logical operation
When clang detects a non-boolean constant in a logical operation it generates a 'constant-logical-operand' warning. In ieee80211_try_rate_control_ops_get() the result of strlen() is used in a logical operation, clang resolves the expression to an (integer) constant at compile time when clang's builtin strlen function is used. Change the condition to check for strlen() > 0 to make the constant operand boolean and thus avoid the warning. Signed-off-by: Matthias Kaehlcke <m...@chromium.org> --- Changes in v2: - Changed expression to strlen() > 0 to make constant operand boolean instead of splitting up condition - Updated commit message net/mac80211/rate.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c index 206698bc93f4..694faf6ab574 100644 --- a/net/mac80211/rate.c +++ b/net/mac80211/rate.c @@ -173,9 +173,11 @@ ieee80211_rate_control_ops_get(const char *name) /* try default if specific alg requested but not found */ ops = ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo); - /* try built-in one if specific alg requested but not found */ - if (!ops && strlen(CONFIG_MAC80211_RC_DEFAULT)) + /* Note: check for > 0 is intentional to avoid clang warning */ + if (!ops && (strlen(CONFIG_MAC80211_RC_DEFAULT) > 0)) + /* try built-in one if specific alg requested but not found */ ops = ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT); + kernel_param_unlock(THIS_MODULE); return ops; -- 2.12.2.715.g7642488e1d-goog
Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation
El Fri, Apr 07, 2017 at 12:51:57AM +0200 Johannes Berg ha dit:
> On Thu, 2017-04-06 at 15:42 -0700, Matthias Kaehlcke wrote:
> >
> > Thanks, it would also require to move the initialization of
> > ieee80211_default_rc_algo into an ifdef. If you can live with such a
> > solution I'm happy to change it.
>
> I think that'd be something I can live with, yeah.
>
> > > git grep 'IS_ENABLED(' | grep '&&'
> >
> > Indeed the warning is not triggered by these constructs. It seems
> > clang only emits the warning when the constant operand is not
> > boolean.
>
> That points to just adding "> 0" to the condition here as another
> alternative solution, I guess? With a comment to make sure it's not
> removed again, that'd seem like the best thing to do.
Good point, that's more digestible. I'll send an updated change soon.
Matthias
Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation
El Thu, Apr 06, 2017 at 11:12:25PM +0200 Johannes Berg ha dit:
> On Thu, 2017-04-06 at 12:24 -0700, Matthias Kaehlcke wrote:
>
> > I agree that the code looks worse :( I hoped to find a fix using a
> > preprocessor condition but wasn't successful.
>
> It's actually easy - just remove the 'default ""' from Kconfig, and
> then the symbol won't be defined at all if it doesn't get a proper
> value. Then you can ifdef the whole thing.
Thanks, it would also require to move the initialization of
ieee80211_default_rc_algo into an ifdef. If you can live with such a
solution I'm happy to change it.
> > Some projects (like Chrome OS) build their kernel with all warnings
> > being treated as errors. Besides changing the 'offending' code the
> > alternatives are to disable the warning completely or to tell clang
> > not to use the builtin(s). IMO changing the code is the preferable
> > solution, especially since this is so far the only occurrence of the
> > warning that I have encountered.
> >
> > I used goto instead of nested ifs since other functions in this file
> > use the same pattern. If nested ifs are preferred I can change that.
>
> I don't really buy either argument. The warning is simply bogus - I'm
> very surprised you don't hit it with more similar macros or cases, like
> for example CONFIG_ENABLED(). Try
>
> git grep 'IS_ENABLED(' | grep '&&'
>
> and you'll find lots of places that seem like they should trigger this
> warning.
Indeed the warning is not triggered by these constructs. It seems
clang only emits the warning when the constant operand is not boolean.
> You're advocating to make the code worse - not very significantly in
> this case, but still - just to quiet a compiler warning.
For Chrome OS we need to quiet the warning in one way or another,
otherwise our builds will fail due to warnings being treated as
errors. I'm reluctant to disable the warning completely since it can
be useful to detect certain errors, e.g. the use of a logical operator
when bitwise was intended.
And yes, in this case I would favor the slight deterioration of a
small section of code over losing a potentially useful check on the
entire kernel code.
I agree that this is a bit of a corner case, the code is definitely
not buggy by itself, ideally clang would detect that the constant is
a result of its own optimization and skip the warning.
Obviously we can always apply a patch locally, but ideally we try to
upstream changes that seem of general use so that others and our
future selves can benefit from it.
I have no intention to insist on this, we can live with a local patch
if you don't think it is useful.
Cheers
Matthias
[PATCH] ath9k: Add cast to u8 to FREQ2FBIN macro
The macro results are assigned to u8 variables/fields. Adding the cast fixes plenty of clang warnings about "implicit conversion from 'int' to 'u8'". Signed-off-by: Matthias Kaehlcke <m...@chromium.org> --- drivers/net/wireless/ath/ath9k/eeprom.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h b/drivers/net/wireless/ath/ath9k/eeprom.h index 30bf722e33ed..31390af6c33e 100644 --- a/drivers/net/wireless/ath/ath9k/eeprom.h +++ b/drivers/net/wireless/ath/ath9k/eeprom.h @@ -106,7 +106,7 @@ #define AR9285_RDEXT_DEFAULT0x1F #define ATH9K_POW_SM(_r, _s) (((_r) & 0x3f) << (_s)) -#define FREQ2FBIN(x, y)((y) ? ((x) - 2300) : (((x) - 4800) / 5)) +#define FREQ2FBIN(x, y)(u8)((y) ? ((x) - 2300) : (((x) - 4800) / 5)) #define FBIN2FREQ(x, y)((y) ? (2300 + x) : (4800 + 5 * x)) #define ath9k_hw_use_flash(_ah)(!(_ah->ah_flags & AH_USE_EEPROM)) -- 2.12.2.715.g7642488e1d-goog
Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation
Hi Johannes, thanks for your comments El Thu, Apr 06, 2017 at 09:11:18PM +0200 Johannes Berg ha dit: > On Thu, 2017-04-06 at 11:56 -0700, Matthias Kaehlcke wrote: > > Clang raises a warning about the expression 'strlen(CONFIG_XXX)' > > being > > used in a logical operation. Clangs' builtin strlen function resolves > > the > > expression to a constant at compile time, which causes clang to > > generate > > a 'constant-logical-operand' warning. > > > > Split the if statement in two to avoid using the const expression in > > a logical operation. > > > I don't really see all much point in doing this for the warning's > sake... hopefully it doesn't actually generate worse code, but I think > the code ends up looking worse and people will forever wonder what the > goto is really doing there. I agree that the code looks worse :( I hoped to find a fix using a preprocessor condition but wasn't successful. Some projects (like Chrome OS) build their kernel with all warnings being treated as errors. Besides changing the 'offending' code the alternatives are to disable the warning completely or to tell clang not to use the builtin(s). IMO changing the code is the preferable solution, especially since this is so far the only occurrence of the warning that I have encountered. I used goto instead of nested ifs since other functions in this file use the same pattern. If nested ifs are preferred I can change that. Cheers Matthias
[PATCH] mac80211: Fix clang warning about constant operand in logical operation
Clang raises a warning about the expression 'strlen(CONFIG_XXX)' being used in a logical operation. Clangs' builtin strlen function resolves the expression to a constant at compile time, which causes clang to generate a 'constant-logical-operand' warning. Split the if statement in two to avoid using the const expression in a logical operation. Signed-off-by: Matthias Kaehlcke <m...@chromium.org> --- net/mac80211/rate.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c index 206698bc93f4..68ff202d6380 100644 --- a/net/mac80211/rate.c +++ b/net/mac80211/rate.c @@ -173,9 +173,14 @@ ieee80211_rate_control_ops_get(const char *name) /* try default if specific alg requested but not found */ ops = ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo); + if (ops) + goto unlock; + /* try built-in one if specific alg requested but not found */ - if (!ops && strlen(CONFIG_MAC80211_RC_DEFAULT)) + if (strlen(CONFIG_MAC80211_RC_DEFAULT)) ops = ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT); + +unlock: kernel_param_unlock(THIS_MODULE); return ops; -- 2.12.2.715.g7642488e1d-goog
[PATCH v2] cfg80211: Fix array-bounds warning in fragment copy
__ieee80211_amsdu_copy_frag intentionally initializes a pointer to
array[-1] to increment it later to valid values. clang rightfully
generates an array-bounds warning on the initialization statement.
Initialize the pointer to array[0] and change the algorithm from
increment before to increment after consume.
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
net/wireless/util.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 68e5f2ecee1a..52795ae5337f 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -659,7 +659,7 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
int offset, int len)
{
struct skb_shared_info *sh = skb_shinfo(skb);
- const skb_frag_t *frag = >frags[-1];
+ const skb_frag_t *frag = >frags[0];
struct page *frag_page;
void *frag_ptr;
int frag_len, frag_size;
@@ -672,10 +672,10 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
while (offset >= frag_size) {
offset -= frag_size;
- frag++;
frag_page = skb_frag_page(frag);
frag_ptr = skb_frag_address(frag);
frag_size = skb_frag_size(frag);
+ frag++;
}
frag_ptr += offset;
@@ -687,12 +687,12 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
len -= cur_len;
while (len > 0) {
- frag++;
frag_len = skb_frag_size(frag);
cur_len = min(len, frag_len);
__frame_add_frag(frame, skb_frag_page(frag),
skb_frag_address(frag), cur_len, frag_len);
len -= cur_len;
+ frag++;
}
}
--
2.12.1.578.ge9c3154ca4-goog
Re: [PATCH] cfg80211: Fix array-bounds warning in fragment copy
El Mon, Mar 27, 2017 at 12:47:59PM +0200 Johannes Berg ha dit:
> On Fri, 2017-03-24 at 18:06 -0700, Matthias Kaehlcke wrote:
> > __ieee80211_amsdu_copy_frag intentionally initializes a pointer to
> > array[-1] to increment it later to valid values. clang rightfully
> > generates an array-bounds warning on the initialization statement.
> > Work around this by initializing the pointer to array[0] and
> > decrementing it later, which allows to leave the rest of the
> > algorithm untouched.
> >
> > Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
> > ---
> > net/wireless/util.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/wireless/util.c b/net/wireless/util.c
> > index 68e5f2ecee1a..d3d459e4a070 100644
> > --- a/net/wireless/util.c
> > +++ b/net/wireless/util.c
> > @@ -659,7 +659,7 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb,
> > struct sk_buff *frame,
> > int offset, int len)
> > {
> > struct skb_shared_info *sh = skb_shinfo(skb);
> > - const skb_frag_t *frag = >frags[-1];
> > + const skb_frag_t *frag = >frags[0];
> > struct page *frag_page;
> > void *frag_ptr;
> > int frag_len, frag_size;
> > @@ -669,6 +669,7 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb,
> > struct sk_buff *frame,
> > frag_page = virt_to_head_page(skb->head);
> > frag_ptr = skb->data;
> > frag_size = head_size;
> > + frag--;
>
> Isn't it just a question of time until the compiler will see through
> this trick and warn about it?
Maybe.
Actually it seems the algorithm can be easily adapted to increment the
pointer after consumption, which is clearer anyway. I will give this a
shot. I'm not sure how to exercise the code path for testing and would
appreciate help on this end.
Matthias
[PATCH] cfg80211: Fix array-bounds warning in fragment copy
__ieee80211_amsdu_copy_frag intentionally initializes a pointer to
array[-1] to increment it later to valid values. clang rightfully
generates an array-bounds warning on the initialization statement.
Work around this by initializing the pointer to array[0] and
decrementing it later, which allows to leave the rest of the
algorithm untouched.
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
net/wireless/util.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 68e5f2ecee1a..d3d459e4a070 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -659,7 +659,7 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
int offset, int len)
{
struct skb_shared_info *sh = skb_shinfo(skb);
- const skb_frag_t *frag = >frags[-1];
+ const skb_frag_t *frag = >frags[0];
struct page *frag_page;
void *frag_ptr;
int frag_len, frag_size;
@@ -669,6 +669,7 @@ __ieee80211_amsdu_copy_frag(struct sk_buff *skb, struct
sk_buff *frame,
frag_page = virt_to_head_page(skb->head);
frag_ptr = skb->data;
frag_size = head_size;
+ frag--;
while (offset >= frag_size) {
offset -= frag_size;
--
2.12.1.578.ge9c3154ca4-goog
[PATCH] PLIP driver: convert killed_timer_sem to completion
PLIP driver: convert the semaphore killed_timer_sem to
a completion
Signed-off-by: Matthias Kaehlcke [EMAIL PROTECTED]
--
diff --git a/drivers/net/plip.c b/drivers/net/plip.c
index 57c9866..fee3d7b 100644
--- a/drivers/net/plip.c
+++ b/drivers/net/plip.c
@@ -106,6 +106,7 @@ static const char version[] = NET3 PLIP version
2.4-parport [EMAIL PROTECTED]
#include linux/if_plip.h
#include linux/workqueue.h
#include linux/spinlock.h
+#include linux/completion.h
#include linux/parport.h
#include linux/bitops.h
@@ -114,7 +115,6 @@ static const char version[] = NET3 PLIP version
2.4-parport [EMAIL PROTECTED]
#include asm/system.h
#include asm/irq.h
#include asm/byteorder.h
-#include asm/semaphore.h
/* Maximum number of devices to support. */
#define PLIP_MAX 8
@@ -221,7 +221,7 @@ struct net_local {
int should_relinquish;
spinlock_t lock;
atomic_t kill_timer;
- struct semaphore killed_timer_sem;
+ struct completion killed_timer_cmp;
};
static inline void enable_parport_interrupts (struct net_device *dev)
@@ -385,7 +385,7 @@ plip_timer_bh(struct work_struct *work)
schedule_delayed_work(nl-timer, 1);
}
else {
- up (nl-killed_timer_sem);
+ complete(nl-killed_timer_cmp);
}
}
@@ -1112,9 +1112,9 @@ plip_close(struct net_device *dev)
if (dev-irq == -1)
{
- init_MUTEX_LOCKED (nl-killed_timer_sem);
+ init_completion(nl-killed_timer_cmp);
atomic_set (nl-kill_timer, 1);
- down (nl-killed_timer_sem);
+ wait_for_completion(nl-killed_timer_cmp);
}
#ifdef NOTDEF
--
Matthias Kaehlcke
Linux System Developer
Barcelona
La libertad es como la mañana. Hay quienes esperan dormidos a que
llegue, pero hay quienes desvelan y caminan la noche para alcanzarla
(Subcomandante Marcos)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
