netdev - RE: [PATCH] net: core: Fix to store new mtu setting in netdevice.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 2 Jan 2019 15:24:01 +0530
From:   Murali Krishna Policharla <murali.policharla@...adcom.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>
Cc:     davem@...emloft.net, amritha.nambiar@...el.com,
        ecree@...arflare.com, ktkhai@...tuozzo.com,
        alexander.h.duyck@...el.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: RE: [PATCH] net: core: Fix to store new mtu setting in netdevice.

> Hi Andrew,
>                            Currently net/dsa/slave.c does not have
> ndo_change_mtu function.  But shortly I will be submitting a separate
> patch outside this fix that has ndo_change_mtu function support added to
> DSA switch.  As part of testing the newly added ndo_change_mtu function
> for DSA switch it uncovered that new mtu size is not being updated to
> netdevice structure. This patch fixes this issue and updates  new mtu
size
> to  netdevice structure.
>
> Hope this clarifies, let me know if you need any further information.

> Hi Murali

> Thanks for the explanation.

> However, i looked at the patch you listed as 'fixes'. I don't see what
> came before that setting the mtu when an ndo_change_mtu function is
> provided. It seems to me, if you provide an ndo_change_mtu, it has to
> do the assignment. I don't think you are fixing anything here.

> If you do want to move the assignment into the core, please review all
> the MAC drivers which implement ndo_change_mtu and remove the
> assignment from them.

>	   Thanks
>		Andrew

Hi Andrew/Heiner

                                       Thanks for the feedback. This patch
fixes a case where ndo_change_mtu function is provided but the callback
function is not storing mtu to netdevice structure. In the discussion
thread for this patch it is mentioned that there is a possibility of
ndo_change_mtu function to manipulate the mtu size and then store the
modified value  to netdevice structure. For me it appears this is
incorrect operation since mtu value is being modified and it is not
storing the value that user has configured.  In another  use case that is
discussed user configures mtu value that exceeds the max/min permitted
value. In this case an error should be returned to the user rather than
modifying/storing only the permitted value.  Also any modifications done
to mtu in the callback function for DMA operations/restrictions should be
transparent to user.   As per my understanding ndo_change_mtu callback
function can modify and use the mtu value that suits for the respective
device operation, but only user configured mtu size should be stored in
netdevice structure and this is done in the core driver with this fix.
Please let me know your inputs.


Thanks
Murali