Re: [PATCH v5 1/2] ethtool: provide UAPI for PHY master/slave configuration.

[Michal Kubecek]

On Mon, May 04, 2020 at 10:55:56AM +0200, Oleksij Rempel wrote:
> On Mon, May 04, 2020 at 10:37:34AM +0200, Michal Kubecek wrote:
> > On Mon, May 04, 2020 at 10:04:17AM +0200, Oleksij Rempel wrote:
> > > @Michal,
> > > 
> > > i noticed that linkmodes_fill_reply() some times get not enough
> > > tailroom.
> > > if data->peer_empty == 0
> > > linkmodes_reply_size() size: 476
> > > linkmodes_fill_reply() skb tailroom: 724
> > > 
> > > 
> > > if data->peer_empty == 1
> > > linkmodes_reply_size() size: 216                                      
> > > linkmodes_fill_reply() skb tailroom: 212
> > > 
> > > In the last case i won't be able to attach master_lave state and cfg
> > > fields.
> > > 
> > > It looks like this issue was not introduced by my patches. May be you
> > > have idea, what is missing?
> > 
> > It's my mistake, I'm just not sure why I never ran into this while
> > testing. Please try the patch below.
> 
> thx! it works now:
> [   82.754019] linkmodes_reply_size:103 size: 216
> [   82.758523] linkmodes_fill_reply:117 skb tailroom: 724
> 
> [  126.781892] linkmodes_reply_size:103 size: 476
> [  126.786464] linkmodes_fill_reply:117 skb tailroom: 724

Thank you. It seems so far the natural skb padding was always sufficient
to hide the broken calculation. Some time ago I had a debugging printk
in place which checked calculated an actual size but that was with an
older version of the series and the code has been rewritten few times
before it reached mainline.

I'll run some tests with additional tracing of estimated and actual
message size and if I don't find other problem, I'll submit the fix.

Michal