* Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame
[not found] ` <20200409.102035.13094168508101122.davem@davemloft.net>
@ 2020-04-10 2:19 ` DENG Qingfang
2020-04-10 2:27 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: DENG Qingfang @ 2020-04-10 2:19 UTC (permalink / raw)
To: David Miller
Cc: Andrew Lunn, Florian Fainelli, netdev, Sean Wang, Weijie Gao,
René van Dorst, linux-mediatek, John Crispin,
Vivien Didelot
So, since nothing else uses the mt7530_set_jumbo function, should I
remove the function and just add a single rmw to mt7530_setup?
On Fri, Apr 10, 2020 at 1:20 AM David Miller <davem@davemloft.net> wrote:
>
> From: DENG Qingfang <dqfext@gmail.com>
> Date: Thu, 9 Apr 2020 23:54:09 +0800
>
> > +static void
> > +mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
> > +{
> > + if (kilobytes > 15)
> > + kilobytes = 15;
> ...
> > + /* Enable jumbo frame up to 15 KB */
> > + mt7530_set_jumbo(priv, 15);
>
> You've made the test quite pointless, honestly.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame
2020-04-10 2:19 ` [PATCH net-next] net: dsa: mt7530: enable jumbo frame DENG Qingfang
@ 2020-04-10 2:27 ` Florian Fainelli
2020-04-10 2:49 ` DENG Qingfang
0 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2020-04-10 2:27 UTC (permalink / raw)
To: DENG Qingfang, David Miller
Cc: Andrew Lunn, netdev, Sean Wang, Weijie Gao, René van Dorst,
linux-mediatek, John Crispin, Vivien Didelot
On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> So, since nothing else uses the mt7530_set_jumbo function, should I
> remove the function and just add a single rmw to mt7530_setup?
(please do not top-post on netdev)
There is a proper way to support the MTU configuration for DSA switch
drivers which is:
/*
* MTU change functionality. Switches can also adjust their MRU
through
* this method. By MTU, one understands the SDU (L2 payload) length.
* If the switch needs to account for the DSA tag on the CPU
port, this
* method needs to to do so privately.
*/
int (*port_change_mtu)(struct dsa_switch *ds, int port,
int new_mtu);
int (*port_max_mtu)(struct dsa_switch *ds, int port);
>
> On Fri, Apr 10, 2020 at 1:20 AM David Miller <davem@davemloft.net> wrote:
>>
>> From: DENG Qingfang <dqfext@gmail.com>
>> Date: Thu, 9 Apr 2020 23:54:09 +0800
>>
>>> +static void
>>> +mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
>>> +{
>>> + if (kilobytes > 15)
>>> + kilobytes = 15;
>> ...
>>> + /* Enable jumbo frame up to 15 KB */
>>> + mt7530_set_jumbo(priv, 15);
>>
>> You've made the test quite pointless, honestly.
--
Florian
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame
2020-04-10 2:27 ` Florian Fainelli
@ 2020-04-10 2:49 ` DENG Qingfang
2020-04-10 10:46 ` Vladimir Oltean
0 siblings, 1 reply; 6+ messages in thread
From: DENG Qingfang @ 2020-04-10 2:49 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, netdev, Sean Wang, Weijie Gao, Vivien Didelot,
René van Dorst, linux-mediatek, John Crispin, David Miller
On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > So, since nothing else uses the mt7530_set_jumbo function, should I
> > remove the function and just add a single rmw to mt7530_setup?
>
> (please do not top-post on netdev)
>
> There is a proper way to support the MTU configuration for DSA switch
> drivers which is:
>
> /*
> * MTU change functionality. Switches can also adjust their MRU
> through
> * this method. By MTU, one understands the SDU (L2 payload) length.
> * If the switch needs to account for the DSA tag on the CPU
> port, this
> * method needs to to do so privately.
> */
> int (*port_change_mtu)(struct dsa_switch *ds, int port,
> int new_mtu);
> int (*port_max_mtu)(struct dsa_switch *ds, int port);
MT7530 does not support configuring jumbo frame per-port
The register affects globally
>
> >
> > On Fri, Apr 10, 2020 at 1:20 AM David Miller <davem@davemloft.net> wrote:
> >>
> >> From: DENG Qingfang <dqfext@gmail.com>
> >> Date: Thu, 9 Apr 2020 23:54:09 +0800
> >>
> >>> +static void
> >>> +mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
> >>> +{
> >>> + if (kilobytes > 15)
> >>> + kilobytes = 15;
> >> ...
> >>> + /* Enable jumbo frame up to 15 KB */
> >>> + mt7530_set_jumbo(priv, 15);
> >>
> >> You've made the test quite pointless, honestly.
>
> --
> Florian
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame
2020-04-10 2:49 ` DENG Qingfang
@ 2020-04-10 10:46 ` Vladimir Oltean
2020-04-14 3:03 ` DENG Qingfang
0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Oltean @ 2020-04-10 10:46 UTC (permalink / raw)
To: DENG Qingfang
Cc: Andrew Lunn, Florian Fainelli, netdev, Sean Wang, Weijie Gao,
Vivien Didelot, René van Dorst,
moderated list:ARM/Mediatek SoC support, John Crispin,
David Miller
Hi Qingfang,
On Fri, 10 Apr 2020 at 05:51, DENG Qingfang <dqfext@gmail.com> wrote:
>
> On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >
> >
> >
> > On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > > So, since nothing else uses the mt7530_set_jumbo function, should I
> > > remove the function and just add a single rmw to mt7530_setup?
> >
> > (please do not top-post on netdev)
> >
> > There is a proper way to support the MTU configuration for DSA switch
> > drivers which is:
> >
> > /*
> > * MTU change functionality. Switches can also adjust their MRU
> > through
> > * this method. By MTU, one understands the SDU (L2 payload) length.
> > * If the switch needs to account for the DSA tag on the CPU
> > port, this
> > * method needs to to do so privately.
> > */
> > int (*port_change_mtu)(struct dsa_switch *ds, int port,
> > int new_mtu);
> > int (*port_max_mtu)(struct dsa_switch *ds, int port);
>
> MT7530 does not support configuring jumbo frame per-port
> The register affects globally
>
> >
> > --
> > Florian
This is a bit more tricky, but I think you can still deal with it
using the port_change_mtu functionality. Basically it is only a
problem when the other ports are standalone - otherwise the
dsa_bridge_mtu_normalization function should kick in.
So if you implement port_change_mtu, you should do something along the lines of:
for (i = 0; i < MT7530_NUM_PORTS; i++) {
struct net_device *slave;
if (!dsa_is_user_port(ds, i))
continue;
slave = ds->ports[i].slave;
slave->mtu = new_mtu;
}
to update the MTU known by the stack for all net devices.
Hope this helps,
-Vladimir
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame
2020-04-10 10:46 ` Vladimir Oltean
@ 2020-04-14 3:03 ` DENG Qingfang
2020-04-14 9:38 ` Vladimir Oltean
0 siblings, 1 reply; 6+ messages in thread
From: DENG Qingfang @ 2020-04-14 3:03 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, netdev, Sean Wang, Weijie Gao,
Vivien Didelot, René van Dorst,
moderated list:ARM/Mediatek SoC support, John Crispin,
David Miller
On Fri, Apr 10, 2020 at 6:46 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Qingfang,
>
> On Fri, 10 Apr 2020 at 05:51, DENG Qingfang <dqfext@gmail.com> wrote:
> >
> > On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > >
> > >
> > >
> > > On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > > > So, since nothing else uses the mt7530_set_jumbo function, should I
> > > > remove the function and just add a single rmw to mt7530_setup?
> > >
> > > (please do not top-post on netdev)
> > >
> > > There is a proper way to support the MTU configuration for DSA switch
> > > drivers which is:
> > >
> > > /*
> > > * MTU change functionality. Switches can also adjust their MRU
> > > through
> > > * this method. By MTU, one understands the SDU (L2 payload) length.
> > > * If the switch needs to account for the DSA tag on the CPU
> > > port, this
> > > * method needs to to do so privately.
> > > */
> > > int (*port_change_mtu)(struct dsa_switch *ds, int port,
> > > int new_mtu);
> > > int (*port_max_mtu)(struct dsa_switch *ds, int port);
> >
> > MT7530 does not support configuring jumbo frame per-port
> > The register affects globally
> >
> > >
> > > --
> > > Florian
>
> This is a bit more tricky, but I think you can still deal with it
> using the port_change_mtu functionality. Basically it is only a
> problem when the other ports are standalone - otherwise the
> dsa_bridge_mtu_normalization function should kick in.
> So if you implement port_change_mtu, you should do something along the lines of:
>
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> struct net_device *slave;
>
> if (!dsa_is_user_port(ds, i))
> continue;
>
> slave = ds->ports[i].slave;
>
> slave->mtu = new_mtu;
> }
>
> to update the MTU known by the stack for all net devices.
Should we warn users that all ports will be affected?
>
> Hope this helps,
> -Vladimir
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame
2020-04-14 3:03 ` DENG Qingfang
@ 2020-04-14 9:38 ` Vladimir Oltean
0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Oltean @ 2020-04-14 9:38 UTC (permalink / raw)
To: DENG Qingfang
Cc: Andrew Lunn, Florian Fainelli, netdev, Sean Wang, Weijie Gao,
Vivien Didelot, René van Dorst,
moderated list:ARM/Mediatek SoC support, John Crispin,
David Miller
On Tue, 14 Apr 2020 at 06:03, DENG Qingfang <dqfext@gmail.com> wrote:
>
> On Fri, Apr 10, 2020 at 6:46 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > Hi Qingfang,
> >
> > On Fri, 10 Apr 2020 at 05:51, DENG Qingfang <dqfext@gmail.com> wrote:
> > >
> > > On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > >
> > > >
> > > >
> > > > On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > > > > So, since nothing else uses the mt7530_set_jumbo function, should I
> > > > > remove the function and just add a single rmw to mt7530_setup?
> > > >
> > > > (please do not top-post on netdev)
> > > >
> > > > There is a proper way to support the MTU configuration for DSA switch
> > > > drivers which is:
> > > >
> > > > /*
> > > > * MTU change functionality. Switches can also adjust their MRU
> > > > through
> > > > * this method. By MTU, one understands the SDU (L2 payload) length.
> > > > * If the switch needs to account for the DSA tag on the CPU
> > > > port, this
> > > > * method needs to to do so privately.
> > > > */
> > > > int (*port_change_mtu)(struct dsa_switch *ds, int port,
> > > > int new_mtu);
> > > > int (*port_max_mtu)(struct dsa_switch *ds, int port);
> > >
> > > MT7530 does not support configuring jumbo frame per-port
> > > The register affects globally
> > >
> > > >
> > > > --
> > > > Florian
> >
> > This is a bit more tricky, but I think you can still deal with it
> > using the port_change_mtu functionality. Basically it is only a
> > problem when the other ports are standalone - otherwise the
> > dsa_bridge_mtu_normalization function should kick in.
> > So if you implement port_change_mtu, you should do something along the lines of:
> >
> > for (i = 0; i < MT7530_NUM_PORTS; i++) {
> > struct net_device *slave;
> >
> > if (!dsa_is_user_port(ds, i))
> > continue;
> >
> > slave = ds->ports[i].slave;
> >
> > slave->mtu = new_mtu;
> > }
> >
> > to update the MTU known by the stack for all net devices.
> Should we warn users that all ports will be affected?
> >
> > Hope this helps,
> > -Vladimir
Unless I'm missing something, all ports are affected anyway, so
changing the MTU _is_ informing users that all switch ports are
affected.
Thanks,
-Vladimir
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-14 9:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200409155409.12043-1-dqfext@gmail.com>
[not found] ` <20200409.102035.13094168508101122.davem@davemloft.net>
2020-04-10 2:19 ` [PATCH net-next] net: dsa: mt7530: enable jumbo frame DENG Qingfang
2020-04-10 2:27 ` Florian Fainelli
2020-04-10 2:49 ` DENG Qingfang
2020-04-10 10:46 ` Vladimir Oltean
2020-04-14 3:03 ` DENG Qingfang
2020-04-14 9:38 ` Vladimir Oltean
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).