Re: [PATCH net] net/flow: fix fc->percpu NULL pointer dereference

2017-06-09 Thread Steffen Klassert
On Fri, Jun 09, 2017 at 04:23:01PM +0800, Hangbin Liu wrote:
> Hi Steffen,
> 
> BTW, If we put the check in xfrm_policy_flush(), we can prevent it earlier.
> But If we put the check in flow_cache_percpu_empty(), we can prevent
> other functions set fc->percpu to NULL, although not much possible : )
> 
> So I'm not quite sure whether we should put the check in
> flow_cache_percpu_empty() or in xfrm_policy_flush().

Can't we just call xfrm_policy_fini() first and then flow_cache_fini()?



Re: [PATCH v2 7/8] net: mvmdio: add xmdio support

2017-06-09 Thread Antoine Tenart
On Thu, Jun 08, 2017 at 09:42:21AM -0700, Florian Fainelli wrote:
> On 06/08/2017 02:26 AM, Antoine Tenart wrote:
> > This patch adds the xMDIO interface support in the mvmdio driver. This
> > interface is used in Ethernet controllers on Marvell 370, 7k and 8k (as
> > of now). The xSMI interface supported by this driver complies with the
> > IEEE 802.3 clause 45 (while the SMI interface complies with the clause
> > 22). The xSMI interface is used by 10GbE devices.
> 
> In the previous version you were properly defining a new compatibles
> strings for xmdio, but now you don't and instead you runtime select the
> operations based on whether MII_ADDR_C45 is set in the register which is
> fine from a functional perspective.
> 
> If I get this right, the xMDIO controller is actually a superset of the
> MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to
> preform C45 accesses?

This is also a mistake. It's not a superset as the register offsets are
different. So we can't use the same smi operations in both cases. We
would need dedicated xmdio smi operations, but I don't think there is a
board where a c22 PHY is connected to the xMDIO interface.

I'll add smi and xsmi flags and return -ENOTSUPP based on the
MII_ADDR_C45 bit and flags combination. If the need to support smi
operations on the xMDIO bus arise it will be fairly easy to implement in
the future. I'll rename the ops functions as well to differentiate mdio
and xmdio operations.

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx

2017-06-09 Thread David Laight
From: Mintz, Yuval
> Sent: 09 June 2017 08:52
> > From: David Laight [mailto:david.lai...@aculab.com]
> > Sent: Friday, June 09, 2017 10:28 AM
> > To: 'David Miller' ; Mintz, Yuval
> > 
> > Cc: netdev@vger.kernel.org; linux-r...@vger.kernel.org; Kalderon, Michal
> > 
> > Subject: RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx
> >
> > From: David Miller
> > > Sent: 09 June 2017 00:24
> > >
> > > From: Yuval Mintz 
> > > Date: Thu, 8 Jun 2017 19:13:16 +0300
> > >
> > > > @@ -67,6 +79,21 @@ struct qed_ll2_stats {
> > > > u64 sent_bcast_pkts;
> > > >  };
> > > >
> > > > +struct qed_ll2_tx_pkt_info {
> > > > +   u8 num_of_bds;
> > > > +   u16 vlan;
> > > > +   u8 bd_flags;
> > > > +   u16 l4_hdr_offset_w;/* from start of packet */
> > > > +   enum qed_ll2_tx_dest tx_dest;
> > > > +   enum qed_ll2_roce_flavor_type qed_roce_flavor;
> > > > +   dma_addr_t first_frag;
> > > > +   u16 first_frag_len;
> > > > +   bool enable_ip_cksum;
> > > > +   bool enable_l4_cksum;
> > > > +   bool calc_ip_len;
> > > > +   void *cookie;
> > > > +};
> > > > +
> > >
> > > This layout is extremely inefficient, with lots of padding in between
> > > struct members.
> > >
> > > Group small u8 members and u16 members together so that they consume
> > > full 32-bit areas so you can eliminate all of the padding.
> >
> > I'd also query the use of u16 sizes/lengths, any arithmetic on u16 (and u8)
> > variables is likely to generate extra code (on non-x86).
> > You are using 32 bits for the 'enum' - I bet the values fit in 8 bits, so 
> > aren't
> > really worried about size.
> >
> > If size did matter you can easily get the above down to 32 bytes.
> 
> You're right, and that's exactly the point -  since this is not data-path 
> critical
> I don't see why the size/efficiency should matter [greatly].

It is just good practise so that it happens automatically when it does matter.
Just swapping 'vlan' and 'bd_flags' would make it look much better.

David



Re: [PATCH net] net/flow: fix fc->percpu NULL pointer dereference

2017-06-09 Thread Hangbin Liu
Hi Steffen,

BTW, If we put the check in xfrm_policy_flush(), we can prevent it earlier.
But If we put the check in flow_cache_percpu_empty(), we can prevent
other functions set fc->percpu to NULL, although not much possible : )

So I'm not quite sure whether we should put the check in
flow_cache_percpu_empty() or in xfrm_policy_flush().

Do you have any suggestion?

Thanks
Hangbin

2017-06-09 16:13 GMT+08:00 Hangbin Liu :
> Now we will force to do garbage collection if any policy removed in
> xfrm_policy_flush(). But during xfrm_net_exit(). We call flow_cache_fini()
> first and set set fc->percpu to NULL. Then after we call xfrm_policy_fini()
> -> frxm_policy_flush() -> flow_cache_flush(), we will get NULL pointer
> dereference when check percpu_empty. The code path looks like:
>
> flow_cache_fini()
>   - fc->percpu = NULL
> xfrm_policy_fini()
>   - xfrm_policy_flush()
> - xfrm_garbage_collect()
>   - flow_cache_flush()
> - flow_cache_percpu_empty()
>   - fcp = per_cpu_ptr(fc->percpu, cpu)
>
> To reproduce, just add ipsec in netns and then remove the netns.
>
> Fixes: 35db06912189 ("xfrm: do the garbage collection after flushing policy")
> Signed-off-by: Hangbin Liu 
> ---
>  net/core/flow.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/flow.c b/net/core/flow.c
> index f7f5d19..321fc53 100644
> --- a/net/core/flow.c
> +++ b/net/core/flow.c
> @@ -332,10 +332,13 @@ static int flow_cache_percpu_empty(struct flow_cache 
> *fc, int cpu)
> struct flow_cache_percpu *fcp;
> unsigned int i;
>
> -   fcp = per_cpu_ptr(fc->percpu, cpu);
> -   for (i = 0; i < flow_cache_hash_size(fc); i++)
> -   if (!hlist_empty(&fcp->hash_table[i]))
> -   return 0;
> +   if (fc->percpu) {
> +   fcp = per_cpu_ptr(fc->percpu, cpu);
> +   for (i = 0; i < flow_cache_hash_size(fc); i++)
> +   if (!hlist_empty(&fcp->hash_table[i]))
> +   return 0;
> +   }
> +
> return 1;
>  }
>
> --
> 2.5.5
>


Re: [PATCH 1/5] net: mvpp2: remove mvpp2_bm_cookie_{build,pool_get}

2017-06-09 Thread kbuild test robot
Hi Thomas,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.12-rc4 next-20170608]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Thomas-Petazzoni/net-mvpp2-fixes-and-cleanups/20170609-083211
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64 

Note: the 
linux-review/Thomas-Petazzoni/net-mvpp2-fixes-and-cleanups/20170609-083211 HEAD 
5a6bfd0022913d8ffcfd0eae9235b9bee844cf53 builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/net/ethernet/marvell/mvpp2.c: In function 'mvpp2_bm_cookie_pool_set':
>> drivers/net/ethernet/marvell/mvpp2.c:3913:26: error: 
>> 'MVPP2_BM_COOKIE_POOL_OFFS' undeclared (first use in this function)
 bm = cookie & ~(0xFF << MVPP2_BM_COOKIE_POOL_OFFS);
 ^
   drivers/net/ethernet/marvell/mvpp2.c:3913:26: note: each undeclared 
identifier is reported only once for each function it appears in

vim +/MVPP2_BM_COOKIE_POOL_OFFS +3913 drivers/net/ethernet/marvell/mvpp2.c

3f518509 Marcin Wojtas 2014-07-10  3907  
3f518509 Marcin Wojtas 2014-07-10  3908  /* Set pool number in a BM cookie */
3f518509 Marcin Wojtas 2014-07-10  3909  static inline u32 
mvpp2_bm_cookie_pool_set(u32 cookie, int pool)
3f518509 Marcin Wojtas 2014-07-10  3910  {
3f518509 Marcin Wojtas 2014-07-10  3911 u32 bm;
3f518509 Marcin Wojtas 2014-07-10  3912  
3f518509 Marcin Wojtas 2014-07-10 @3913 bm = cookie & ~(0xFF << 
MVPP2_BM_COOKIE_POOL_OFFS);
3f518509 Marcin Wojtas 2014-07-10  3914 bm |= ((pool & 0xFF) << 
MVPP2_BM_COOKIE_POOL_OFFS);
3f518509 Marcin Wojtas 2014-07-10  3915  
3f518509 Marcin Wojtas 2014-07-10  3916 return bm;

:: The code at line 3913 was first introduced by commit
:: 3f518509dedc99f0b755d2ce68d24f610e3a005a ethernet: Add new driver for 
Marvell Armada 375 network unit

:: TO: Marcin Wojtas 
:: CC: David S. Miller 

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


.config.gz
Description: application/gzip


[PATCH net] net/flow: fix fc->percpu NULL pointer dereference

2017-06-09 Thread Hangbin Liu
Now we will force to do garbage collection if any policy removed in
xfrm_policy_flush(). But during xfrm_net_exit(). We call flow_cache_fini()
first and set set fc->percpu to NULL. Then after we call xfrm_policy_fini()
-> frxm_policy_flush() -> flow_cache_flush(), we will get NULL pointer
dereference when check percpu_empty. The code path looks like:

flow_cache_fini()
  - fc->percpu = NULL
xfrm_policy_fini()
  - xfrm_policy_flush()
- xfrm_garbage_collect()
  - flow_cache_flush()
- flow_cache_percpu_empty()
  - fcp = per_cpu_ptr(fc->percpu, cpu)

To reproduce, just add ipsec in netns and then remove the netns.

Fixes: 35db06912189 ("xfrm: do the garbage collection after flushing policy")
Signed-off-by: Hangbin Liu 
---
 net/core/flow.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/core/flow.c b/net/core/flow.c
index f7f5d19..321fc53 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -332,10 +332,13 @@ static int flow_cache_percpu_empty(struct flow_cache *fc, 
int cpu)
struct flow_cache_percpu *fcp;
unsigned int i;
 
-   fcp = per_cpu_ptr(fc->percpu, cpu);
-   for (i = 0; i < flow_cache_hash_size(fc); i++)
-   if (!hlist_empty(&fcp->hash_table[i]))
-   return 0;
+   if (fc->percpu) {
+   fcp = per_cpu_ptr(fc->percpu, cpu);
+   for (i = 0; i < flow_cache_hash_size(fc); i++)
+   if (!hlist_empty(&fcp->hash_table[i]))
+   return 0;
+   }
+
return 1;
 }
 
-- 
2.5.5



Re: [PATCH] wireless: wlcore: spi: remove unnecessary variable

2017-06-09 Thread Kalle Valo
"Gustavo A. R. Silva"  writes:

> Remove unnecessary variable and refactor the code.
>
> Addresses-Coverity-ID: 1365000
> Signed-off-by: Gustavo A. R. Silva 

I'll remove "wireless:" from the prefix.

-- 
Kalle Valo


RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx

2017-06-09 Thread Mintz, Yuval


> -Original Message-
> From: David Laight [mailto:david.lai...@aculab.com]
> Sent: Friday, June 09, 2017 10:28 AM
> To: 'David Miller' ; Mintz, Yuval
> 
> Cc: netdev@vger.kernel.org; linux-r...@vger.kernel.org; Kalderon, Michal
> 
> Subject: RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx
> 
> From: David Miller
> > Sent: 09 June 2017 00:24
> >
> > From: Yuval Mintz 
> > Date: Thu, 8 Jun 2017 19:13:16 +0300
> >
> > > @@ -67,6 +79,21 @@ struct qed_ll2_stats {
> > >   u64 sent_bcast_pkts;
> > >  };
> > >
> > > +struct qed_ll2_tx_pkt_info {
> > > + u8 num_of_bds;
> > > + u16 vlan;
> > > + u8 bd_flags;
> > > + u16 l4_hdr_offset_w;/* from start of packet */
> > > + enum qed_ll2_tx_dest tx_dest;
> > > + enum qed_ll2_roce_flavor_type qed_roce_flavor;
> > > + dma_addr_t first_frag;
> > > + u16 first_frag_len;
> > > + bool enable_ip_cksum;
> > > + bool enable_l4_cksum;
> > > + bool calc_ip_len;
> > > + void *cookie;
> > > +};
> > > +
> >
> > This layout is extremely inefficient, with lots of padding in between
> > struct members.
> >
> > Group small u8 members and u16 members together so that they consume
> > full 32-bit areas so you can eliminate all of the padding.
> 
> I'd also query the use of u16 sizes/lengths, any arithmetic on u16 (and u8)
> variables is likely to generate extra code (on non-x86).
> You are using 32 bits for the 'enum' - I bet the values fit in 8 bits, so 
> aren't
> really worried about size.
> 
> If size did matter you can easily get the above down to 32 bytes.

You're right, and that's exactly the point -  since this is not data-path 
critical
I don't see why the size/efficiency should matter [greatly].


RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx

2017-06-09 Thread David Laight
From: David Miller
> Sent: 09 June 2017 00:24
> 
> From: Yuval Mintz 
> Date: Thu, 8 Jun 2017 19:13:16 +0300
> 
> > @@ -67,6 +79,21 @@ struct qed_ll2_stats {
> > u64 sent_bcast_pkts;
> >  };
> >
> > +struct qed_ll2_tx_pkt_info {
> > +   u8 num_of_bds;
> > +   u16 vlan;
> > +   u8 bd_flags;
> > +   u16 l4_hdr_offset_w;/* from start of packet */
> > +   enum qed_ll2_tx_dest tx_dest;
> > +   enum qed_ll2_roce_flavor_type qed_roce_flavor;
> > +   dma_addr_t first_frag;
> > +   u16 first_frag_len;
> > +   bool enable_ip_cksum;
> > +   bool enable_l4_cksum;
> > +   bool calc_ip_len;
> > +   void *cookie;
> > +};
> > +
> 
> This layout is extremely inefficient, with lots of padding in between
> struct members.
> 
> Group small u8 members and u16 members together so that they consume
> full 32-bit areas so you can eliminate all of the padding.

I'd also query the use of u16 sizes/lengths, any arithmetic on u16 (and u8)
variables is likely to generate extra code (on non-x86).
You are using 32 bits for the 'enum' - I bet the values fit in 8 bits,
so aren't really worried about size.

If size did matter you can easily get the above down to 32 bytes.

David



RE: [B.A.T.M.A.N.] [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice

2017-06-09 Thread 高峰
> From: Sven Eckelmann [mailto:s...@narfation.org]
> Sent: Friday, June 9, 2017 3:23 PM
> Subject: Re: [B.A.T.M.A.N.] [PATCH net] net: batman-adv: Fix possible
memleaks
> when fail to register_netdevice
> 
> On Dienstag, 25. April 2017 20:03:20 CEST gfree.w...@foxmail.com wrote:
> > From: Gao Feng 
> >
> > Because the func batadv_softif_init_late allocate some resources and
> > it would be invoked in register_netdevice. So we need to invoke the
> > func batadv_softif_free instead of free_netdev to cleanup when fail to
> > register_netdevice.
> >
> > Signed-off-by: Gao Feng 
> > ---
> >  net/batman-adv/soft-interface.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/batman-adv/soft-interface.c
> > b/net/batman-adv/soft-interface.c index d042c99..90bf990 100644
> > --- a/net/batman-adv/soft-interface.c
> > +++ b/net/batman-adv/soft-interface.c
> > @@ -1011,7 +1011,7 @@ struct net_device *batadv_softif_create(struct net
> *net, const char *name)
> > if (ret < 0) {
> > pr_err("Unable to register the batman interface '%s': %i\n",
> >name, ret);
> > -   free_netdev(soft_iface);
> > +   batadv_softif_free(soft_iface);
> > return NULL;
> > }
> 
> It looks to me like this change is invalid after David's change [1]. Can
you
> confirm that?
> 
> Thanks,
>   Sven
> 
> [1]
>
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=cf1
> 24db566e6b036b8bcbe8decbed740bdfac8c6
[Gao Feng] 

yes, this change is unnecessary. 

Best Regards
Feng






Re: [B.A.T.M.A.N.] [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice

2017-06-09 Thread Sven Eckelmann
On Dienstag, 25. April 2017 20:03:20 CEST gfree.w...@foxmail.com wrote:
> From: Gao Feng 
> 
> Because the func batadv_softif_init_late allocate some resources and
> it would be invoked in register_netdevice. So we need to invoke the
> func batadv_softif_free instead of free_netdev to cleanup when fail
> to register_netdevice.
> 
> Signed-off-by: Gao Feng 
> ---
>  net/batman-adv/soft-interface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
> index d042c99..90bf990 100644
> --- a/net/batman-adv/soft-interface.c
> +++ b/net/batman-adv/soft-interface.c
> @@ -1011,7 +1011,7 @@ struct net_device *batadv_softif_create(struct net 
> *net, const char *name)
>   if (ret < 0) {
>   pr_err("Unable to register the batman interface '%s': %i\n",
>  name, ret);
> - free_netdev(soft_iface);
> + batadv_softif_free(soft_iface);
>   return NULL;
>   }

It looks to me like this change is invalid after David's change [1]. Can you
confirm that?

Thanks,
Sven

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=cf124db566e6b036b8bcbe8decbed740bdfac8c6

signature.asc
Description: This is a digitally signed message part.


Re: [PATCH net-next 0/8] Bug fixes in ena ethernet driver

2017-06-09 Thread Belgazal, Netanel
My apologies,
I was not aware.
Will make sure this won't happen again.

Regards,
Netanel

From: David Miller 
Sent: Friday, June 9, 2017 2:17 AM
To: Belgazal, Netanel
Cc: netdev@vger.kernel.org; Woodhouse, David; Machulsky, Zorik; Matushevsky, 
Alexander; BSHARA, Said; Wilson, Matt; Liguori, Anthony; Bshara, Nafea; 
Schmeilin, Evgeny
Subject: Re: [PATCH net-next 0/8] Bug fixes in ena ethernet driver

Two parallel patch series to the same driver and targetting the
same GIT tree is extremely undesirable, please don't do this.

Submit one series, and once applied submit the second series.

I'm deleting all of your patches from my queue, please resubmit
things properly.

Thank you.


Re: [PATCH 2/2 net-next] net: stmmac: Improve documentation on AVB parameters

2017-06-09 Thread Giuseppe CAVALLARO

Hi Joao

On 6/8/2017 8:02 PM, Joao Pinto wrote:

This patch fixes the description of the DT AVB parameters and gives
an accurate example. It was also included the base values that were
used to get the example' CBS paremeter values.

Signed-off-by: Joao Pinto 
---
  Documentation/devicetree/bindings/net/stmmac.txt | 24 
  1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt 
b/Documentation/devicetree/bindings/net/stmmac.txt
index c3a7be6..707426d 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -109,10 +109,10 @@ Optional properties:
  [Attention] Queue 0 is reserved for legacy traffic
  and so no AVB is available in this queue.
- Configure Credit Base Shaper (if AVB Mode selected):
-   - snps,send_slope: enable Low Power Interface
-   - snps,idle_slope: unlock on WoL
-   - snps,high_credit: max write outstanding req. limit
-   - snps,low_credit: max read outstanding req. limit
+   - snps,send_slope: Send Slope Credit value
+   - snps,idle_slope:  Idle Slope Credit value
+   - snps,high_credit: High Credit value
+   - snps,low_credit: Low Credit value
- snps,priority: TX queue priority (Range: 0x0 to 0xF)
  Examples:
  
@@ -143,10 +143,18 @@ Examples:
  
  		queue1 {

snps,avb-algorithm;
-   snps,send_slope = <0x1000>;
-   snps,idle_slope = <0x1000>;
-   snps,high_credit = <0x3E800>;
-   snps,low_credit = <0xFFC18000>;
+   /*
+*  Example AVB parameters based on:
+*   Allocated Bandwidth: 40%
+*   Maximum Frame size: 1000 bytes
+*   Maximum Interference size: 1500 bytes
+*   Port Transmit Rate: 8
+*   Scaling Factor: 1024
+*/
+   snps,idle_slope = <0xCCC>;
+   snps,send_slope = <0x1333>;
+   snps,high_credit = <0x4B>;


Thanks for having taken care about this changes, please, as required, 
add a cover-letter
and give more information about these values that can be tuned by user 
and, for example,
the snps,high_credit could be as default = 0xbe4000 that is a reasonable 
value because

comes from 1522 * 8 * 1024 and LOW credit is the two complement.
  ^
frame size ---> maximum is 16

Regards
Peppe


+   snps,low_credit = <0xFFB5>;
snps,priority = <0x1>;
};
};





RE: [PATCH net-next 1/8] qed: LL2 to use packed information for tx

2017-06-09 Thread Mintz, Yuval
> > +struct qed_ll2_tx_pkt_info {
> > +   u8 num_of_bds;
> > +   u16 vlan;
> > +   u8 bd_flags;
> > +   u16 l4_hdr_offset_w;/* from start of packet */
> > +   enum qed_ll2_tx_dest tx_dest;
> > +   enum qed_ll2_roce_flavor_type qed_roce_flavor;
> > +   dma_addr_t first_frag;
> > +   u16 first_frag_len;
> > +   bool enable_ip_cksum;
> > +   bool enable_l4_cksum;
> > +   bool calc_ip_len;
> > +   void *cookie;
> > +};
> > +
> 
> This layout is extremely inefficient, with lots of padding in between struct
> members.
> 
> Group small u8 members and u16 members together so that they consume
> full 32-bit areas so you can eliminate all of the padding.

While I can mend the holes, the total structure size doesn't really change:

struct qed_ll2_tx_pkt_info {
u8 num_of_bds;   /* 0 1 */

/* XXX 1 byte hole, try to pack */

u16vlan; /* 2 2 */
u8 bd_flags; /* 4 1 */

/* XXX 1 byte hole, try to pack */

u16l4_hdr_offset_w;  /* 6 2 */
enum qed_ll2_tx_dest   tx_dest;  /* 8 4 */
enum qed_ll2_roce_flavor_type qed_roce_flavor;   /*12 4 */
dma_addr_t first_frag;   /*16 8 */
u16first_frag_len;   /*24 2 */
bool   enable_ip_cksum;  /*26 1 */
bool   enable_l4_cksum;  /*27 1 */
bool   calc_ip_len;  /*28 1 */

/* XXX 3 bytes hole, try to pack */

void * cookie;   /*32 8 */

/* size: 40, cachelines: 1, members: 12 */
/* sum members: 35, holes: 3, sum holes: 5 */
/* last cacheline: 40 bytes */
};

Becomes:

struct qed_ll2_tx_pkt_info {
void * cookie;   /* 0 8 */
dma_addr_t first_frag;   /* 8 8 */
enum qed_ll2_tx_dest   tx_dest;  /*16 4 */
enum qed_ll2_roce_flavor_type qed_roce_flavor;   /*20 4 */
u16vlan; /*24 2 */
u16l4_hdr_offset_w;  /*26 2 */
u16first_frag_len;   /*28 2 */
u8 num_of_bds;   /*30 1 */
u8 bd_flags; /*31 1 */
bool   enable_ip_cksum;  /*32 1 */
bool   enable_l4_cksum;  /*33 1 */
bool   calc_ip_len;  /*34 1 */

/* size: 40, cachelines: 1, members: 12 */
/* padding: 5 */
/* last cacheline: 40 bytes */
};

I'm going to send the changed version in V2 as as there's no harm to it,
[+ we *can* reduce the size of qed_ll2_comp_rx_data you commented
for patch #2 in series]

But one thing I thought of asking - do we consider layouts of relatively
insignificant structures to be some golden coding standard?


<    1   2   3