[mac80211-next:cfg80211-mac80211-multi-bssid 8/20] ERROR: "__umoddi3" [net/wireless/cfg80211.ko] undefined!

[mac80211-next:cfg80211-mac80211-multi-bssid 8/20] ERROR: "__umoddi3" [net/wireless/cfg80211.ko] undefined!

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git cfg80211-mac80211-multi-bssid
head:   851ae31d34063deb1eae49f5d797a12a5557e832
commit: 0b8fb8235be8be99a197e8d948fc0a2df8dc261a [8/20] cfg80211: Parsing of Multiple BSSID information in scanning
config: i386-randconfig-x0-02091211 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        git checkout 0b8fb8235be8be99a197e8d948fc0a2df8dc261a
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "__umoddi3" [net/wireless/cfg80211.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33212 bytes --]

Re: [mac80211-next:cfg80211-mac80211-multi-bssid 8/20] ERROR: "__umoddi3" [net/wireless/cfg80211.ko] undefined!

[Johannes Berg]

This should fix it, I think?

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b61b71f369c7..cb0c49b369c4 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5488,7 +5488,7 @@ static inline void cfg80211_gen_new_bssid(const u8 *bssid, u8 max_bssid,
        lsb_n = bssid_tmp & ((1 << max_bssid) - 1);
        new_bssid = bssid_tmp;
        new_bssid &= ~((1 << max_bssid) - 1);
-       new_bssid |= (lsb_n + mbssid_index) % (1 << max_bssid);
+       new_bssid |= (lsb_n + mbssid_index) & GENMASK_ULL(max_bssid - 1, 0);
 
        u64_to_ether_addr(new_bssid, new_bssid_addr);
 }

and we probably should write "(1 << max_bssid) - 1" the line above as
the same GENMASK_ULL():

--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5482,13 +5482,14 @@ static inline void cfg80211_gen_new_bssid(const u8 *bssid, u8 max_bssid,
 {
        u64 bssid_tmp, new_bssid;
        u64 lsb_n;
+       u64 mask = GENMASK_ULL(max_bssid - 1, 0);
 
        bssid_tmp = ether_addr_to_u64(bssid);
 
-       lsb_n = bssid_tmp & ((1 << max_bssid) - 1);
+       lsb_n = bssid_tmp & mask;
        new_bssid = bssid_tmp;
-       new_bssid &= ~((1 << max_bssid) - 1);
-       new_bssid |= (lsb_n + mbssid_index) % (1 << max_bssid);
+       new_bssid &= ~mask;
+       new_bssid |= (lsb_n + mbssid_index) & mask;
 
        u64_to_ether_addr(new_bssid, new_bssid_addr);
 }


but maybe the whole thing is more readable as

static inline void cfg80211_gen_new_bssid(const u8 *bssid_addr, u8 max_bssid,
                                          u8 mbssid_index, u8 *new_bssid_addr)
{
        u64 bssid = ether_addr_to_u64(bssid_addr);
        u64 mask = GENMASK_ULL(max_bssid - 1, 0);
        u64 new_bssid;

        new_bssid &= bssid & ~mask;
        new_bssid |= ((bssid & mask) + mbssid_index) & mask;

        u64_to_ether_addr(new_bssid, new_bssid_addr);
}

However, isn't it true that 0 <= mbssid_index < max_bssid? Then the
whole masking isn't really needed at all?

johannes


On Sat, 2019-02-09 at 15:47 +0800, kbuild test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git cfg80211-mac80211-multi-bssid
> head:   851ae31d34063deb1eae49f5d797a12a5557e832
> commit: 0b8fb8235be8be99a197e8d948fc0a2df8dc261a [8/20] cfg80211: Parsing of Multiple BSSID information in scanning
> config: i386-randconfig-x0-02091211 (attached as .config)
> compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
> reproduce:
>         git checkout 0b8fb8235be8be99a197e8d948fc0a2df8dc261a
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
> > > ERROR: "__umoddi3" [net/wireless/cfg80211.ko] undefined!
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [mac80211-next:cfg80211-mac80211-multi-bssid 8/20] ERROR: "__umoddi3" [net/wireless/cfg80211.ko] undefined!

[Jouni Malinen]

On Sat, Feb 09, 2019 at 09:08:20AM +0100, Johannes Berg wrote:

> but maybe the whole thing is more readable as
> 
> static inline void cfg80211_gen_new_bssid(const u8 *bssid_addr, u8 max_bssid,
>                                           u8 mbssid_index, u8 *new_bssid_addr)
> {
>         u64 bssid = ether_addr_to_u64(bssid_addr);
>         u64 mask = GENMASK_ULL(max_bssid - 1, 0);
>         u64 new_bssid;
> 
>         new_bssid &= bssid & ~mask;

That should be "=" not "&="..

>         new_bssid |= ((bssid & mask) + mbssid_index) & mask;
> 
>         u64_to_ether_addr(new_bssid, new_bssid_addr);
> }

but other than that, this version looks much nicer than the other
alternatives.

> However, isn't it true that 0 <= mbssid_index < max_bssid? Then the
> whole masking isn't really needed at all?

0 <= mbssid_index < 2^max_bssid. The transmitted BSSID (i.e., that
bssid_addr argument) is not required to be the first BSSID in the range,
so the masking is needed to cover wraparound for addition modulo
2^mbssid_index when max_bssid LSBs of bssid are not zeros.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [mac80211-next:cfg80211-mac80211-multi-bssid 8/20] ERROR: "__umoddi3" [net/wireless/cfg80211.ko] undefined!

[Johannes Berg]

On Mon, 2019-02-11 at 16:57 +0200, Jouni Malinen wrote:
> On Sat, Feb 09, 2019 at 09:08:20AM +0100, Johannes Berg wrote:
> 
> > but maybe the whole thing is more readable as
> > 
> > static inline void cfg80211_gen_new_bssid(const u8 *bssid_addr, u8 max_bssid,
> >                                           u8 mbssid_index, u8 *new_bssid_addr)
> > {
> >         u64 bssid = ether_addr_to_u64(bssid_addr);
> >         u64 mask = GENMASK_ULL(max_bssid - 1, 0);
> >         u64 new_bssid;
> > 
> >         new_bssid &= bssid & ~mask;
> 
> That should be "=" not "&="..

Yes, good point.

> > However, isn't it true that 0 <= mbssid_index < max_bssid? Then the
> > whole masking isn't really needed at all?
> 
> 0 <= mbssid_index < 2^max_bssid. 

True, sorry.

> The transmitted BSSID (i.e., that
> bssid_addr argument) is not required to be the first BSSID in the range,
> so the masking is needed to cover wraparound for addition modulo
> 2^mbssid_index when max_bssid LSBs of bssid are not zeros.

Ah ok.

Alright, I'll send out a proper patch.

Thanks!

johannes