netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Murali Krishna Policharla <murali.policharla@broadcom.com>
To: Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>
Cc: davem@davemloft.net, amritha.nambiar@intel.com,
	ecree@solarflare.com, ktkhai@virtuozzo.com,
	alexander.h.duyck@intel.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: RE: [PATCH] net: core: Fix to store new mtu setting in netdevice.
Date: Wed, 2 Jan 2019 15:24:01 +0530	[thread overview]
Message-ID: <b6ff97415cd415614393e35793972ccc@mail.gmail.com> (raw)
In-Reply-To: <20190101232443.GA22737@lunn.ch>

> 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

  reply	other threads:[~2019-01-02  9:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-01  6:42 [PATCH] net: core: Fix to store new mtu setting in netdevice Murali Krishna Policharla
2019-01-01  7:54 ` Kirill Tkhai
2019-01-01  9:40   ` Heiner Kallweit
2019-01-01 23:36     ` Andrew Lunn
2019-01-02  6:27       ` Heiner Kallweit
2019-01-01  8:34 ` Andrew Lunn
2019-01-01  9:48   ` Murali Krishna Policharla
2019-01-01 23:24     ` Andrew Lunn
2019-01-02  9:54       ` Murali Krishna Policharla [this message]
2019-01-02 13:16         ` Andrew Lunn
2019-01-03  9:54           ` Murali Krishna Policharla
2019-01-01  9:35 ` Heiner Kallweit
2019-01-01 21:44 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b6ff97415cd415614393e35793972ccc@mail.gmail.com \
    --to=murali.policharla@broadcom.com \
    --cc=alexander.h.duyck@intel.com \
    --cc=amritha.nambiar@intel.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=hkallweit1@gmail.com \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).