| 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
| ||
|
Message-ID: <82a7acf3176c90d9bea773bb4ea365745c1a1971.camel@mediatek.com>
Date: Wed, 26 Oct 2022 19:45:17 +0800
From: haozhe chang <haozhe.chang@...iatek.com>
To: Loic Poulain <loic.poulain@...aro.org>,
"chandrashekar.devegowda@...el.com"
<chandrashekar.devegowda@...el.com>,
"linuxwwan@...el.com" <linuxwwan@...el.com>,
"chiranjeevi.rapolu@...ux.intel.com"
<chiranjeevi.rapolu@...ux.intel.com>,
Haijun Liu (刘海军)
<haijun.liu@...iatek.com>,
"m.chetan.kumar@...ux.intel.com" <m.chetan.kumar@...ux.intel.com>,
"ricardo.martinez@...ux.intel.com" <ricardo.martinez@...ux.intel.com>,
"ryazanov.s.a@...il.com" <ryazanov.s.a@...il.com>,
"johannes@...solutions.net" <johannes@...solutions.net>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Lambert Wang (王伟)
<Lambert.Wang@...iatek.com>,
"Xiayu Zhang (张夏宇)"
<Xiayu.Zhang@...iatek.com>, <srv_heupstream@...iatek.com>
Subject: Re: [PATCH] wwan: core: Support slicing in port TX flow of WWAN
subsystem
On Wed, 2022-10-26 at 15:28 +0800, Loic Poulain wrote:
> Hi Haozhe,
>
> On Wed, 26 Oct 2022 at 03:16, <haozhe.chang@...iatek.com> wrote:
> >
> > From: haozhe chang <haozhe.chang@...iatek.com>
> >
> > wwan_port_fops_write inputs the SKB parameter to the TX callback of
> > the WWAN device driver. However, the WWAN device (e.g., t7xx) may
> > have an MTU less than the size of SKB, causing the TX buffer to be
> > sliced and copied once more in the WWAN device driver.
>
> The benefit of putting data in an skb is that it is easy to
> manipulate, so not sure why there is an additional copy in the first
> place. Isn't possible for the t7xx driver to consume the skb
> progressively (without intermediate copy), according to its own MTU
> limitation?
>
t7xx driver needs to add metadata to the SKB head for each fragment, so
the driver has to allocate a new buffer to copy data(skb_put_data) and
insert metadata.
Providing the option to slice in common layer benefits varieties of
devices with different DMA capabilities. The patch is also compatible
with existing WWAN devices.
> >
> > This patch implements the slicing in the WWAN subsystem and gives
> > the WWAN devices driver the option to slice(by chunk) or not. By
> > doing so, the additional memory copy is reduced.
> >
> > Meanwhile, this patch gives WWAN devices driver the option to
> > reserve
> > headroom in SKB for the device-specific metadata.
> >
> > Signed-off-by: haozhe chang <haozhe.chang@...iatek.com>
> > ---
> > drivers/net/wwan/t7xx/t7xx_port_wwan.c | 41 ++++++++++++--------
> > ---
> > drivers/net/wwan/wwan_core.c | 45 ++++++++++++++++++--
> > ------
> > include/linux/wwan.h | 5 ++-
> > 3 files changed, 56 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/net/wwan/t7xx/t7xx_port_wwan.c
> > b/drivers/net/wwan/t7xx/t7xx_port_wwan.c
> > index 33931bfd78fd..5e8589582121 100644
> > --- a/drivers/net/wwan/t7xx/t7xx_port_wwan.c
> > +++ b/drivers/net/wwan/t7xx/t7xx_port_wwan.c
> > @@ -54,13 +54,12 @@ static void t7xx_port_ctrl_stop(struct
> > wwan_port *port)
> > static int t7xx_port_ctrl_tx(struct wwan_port *port, struct
> > sk_buff *skb)
> > {
> > struct t7xx_port *port_private =
> > wwan_port_get_drvdata(port);
> > - size_t len, offset, chunk_len = 0, txq_mtu = CLDMA_MTU;
> > const struct t7xx_port_conf *port_conf;
> > struct t7xx_fsm_ctl *ctl;
> > enum md_state md_state;
> > + int ret;
> >
> > - len = skb->len;
> > - if (!len || !port_private->chan_enable)
> > + if (!port_private->chan_enable)
> > return -EINVAL;
> >
> > port_conf = port_private->port_conf;
> > @@ -72,33 +71,33 @@ static int t7xx_port_ctrl_tx(struct wwan_port
> > *port, struct sk_buff *skb)
> > return -ENODEV;
> > }
> >
> > - for (offset = 0; offset < len; offset += chunk_len) {
> > - struct sk_buff *skb_ccci;
> > - int ret;
> > -
> > - chunk_len = min(len - offset, txq_mtu -
> > sizeof(struct ccci_header));
> > - skb_ccci = t7xx_port_alloc_skb(chunk_len);
> > - if (!skb_ccci)
> > - return -ENOMEM;
> > -
> > - skb_put_data(skb_ccci, skb->data + offset,
> > chunk_len);
> > - ret = t7xx_port_send_skb(port_private, skb_ccci, 0,
> > 0);
> > - if (ret) {
> > - dev_kfree_skb_any(skb_ccci);
> > - dev_err(port_private->dev, "Write error on
> > %s port, %d\n",
> > - port_conf->name, ret);
> > - return ret;
> > - }
> > + ret = t7xx_port_send_skb(port_private, skb, 0, 0);
> > + if (ret) {
> > + dev_err(port_private->dev, "Write error on %s port,
> > %d\n",
> > + port_conf->name, ret);
> > + return ret;
> > }
> > -
> > dev_kfree_skb(skb);
> > +
> > return 0;
> > }
> >
> > +static size_t t7xx_port_get_tx_rsvd_headroom(struct wwan_port
> > *port)
> > +{
> > + return sizeof(struct ccci_header);
> > +}
> > +
> > +static size_t t7xx_port_get_tx_chunk_len(struct wwan_port *port)
> > +{
> > + return CLDMA_MTU - sizeof(struct ccci_header);
> > +}
> > +
> > static const struct wwan_port_ops wwan_ops = {
> > .start = t7xx_port_ctrl_start,
> > .stop = t7xx_port_ctrl_stop,
> > .tx = t7xx_port_ctrl_tx,
> > + .get_tx_rsvd_headroom = t7xx_port_get_tx_rsvd_headroom,
>
> Can't we have a simple 'skb_headroom' or 'needed_headroom' member
> here?
>
OK, I will change it in patch v2.
> > + .get_tx_chunk_len = t7xx_port_get_tx_chunk_len,
> > };
> >
Powered by blists - more mailing lists