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
| ||
|
Date: Mon, 21 Nov 2022 11:01:57 +0000 From: Haozhe Chang (常浩哲) <Haozhe.Chang@...iatek.com> To: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org> CC: "stephan@...hold.net" <stephan@...hold.net>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>, "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, "linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>, "linuxwwan@...el.com" <linuxwwan@...el.com>, "m.chetan.kumar@...el.com" <m.chetan.kumar@...el.com>, "oneukum@...e.com" <oneukum@...e.com>, Hua Yang (杨华) <Hua.Yang@...iatek.com>, "chiranjeevi.rapolu@...ux.intel.com" <chiranjeevi.rapolu@...ux.intel.com>, Haijun Liu (刘海军) <haijun.liu@...iatek.com>, "ryazanov.s.a@...il.com" <ryazanov.s.a@...il.com>, "kuba@...nel.org" <kuba@...nel.org>, "loic.poulain@...aro.org" <loic.poulain@...aro.org>, "edumazet@...gle.com" <edumazet@...gle.com>, "johannes@...solutions.net" <johannes@...solutions.net>, "chandrashekar.devegowda@...el.com" <chandrashekar.devegowda@...el.com>, "pabeni@...hat.com" <pabeni@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "shangxiaojing@...wei.com" <shangxiaojing@...wei.com>, Lambert Wang (王伟) <Lambert.Wang@...iatek.com>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>, "davem@...emloft.net" <davem@...emloft.net>, "ricardo.martinez@...ux.intel.com" <ricardo.martinez@...ux.intel.com>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, Xiayu Zhang (张夏宇) <Xiayu.Zhang@...iatek.com> Subject: Re: [PATCH v3] wwan: core: Support slicing in port TX flow of WWAN subsystem On Mon, 2022-11-14 at 12:40 +0100, gregkh@...uxfoundation.org wrote: > On Mon, Nov 14, 2022 at 11:23:19AM +0000, Haozhe Chang (常浩哲) wrote: > > Hi Greg Kroah-Hartman > > > > On Fri, 2022-11-11 at 16:02 +0100, Greg Kroah-Hartman wrote: > > > On Fri, Nov 11, 2022 at 06:08:36PM +0800, > > > 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. > > > > > > > > This patch implements the slicing in the WWAN subsystem and > > > > gives > > > > the WWAN devices driver the option to slice(by frag_len) or > > > > not. By > > > > doing so, the additional memory copy is reduced. > > > > > > > > Meanwhile, this patch gives WWAN devices driver the option to > > > > reserve > > > > headroom in fragments for the device-specific metadata. > > > > > > > > Signed-off-by: haozhe chang <haozhe.chang@...iatek.com> > > > > > > > > --- > > > > Changes in v2 > > > > -send fragments to device driver by skb frag_list. > > > > > > > > Changes in v3 > > > > -move frag_len and headroom_len setting to wwan_create_port. > > > > --- > > > > drivers/net/wwan/iosm/iosm_ipc_port.c | 3 +- > > > > drivers/net/wwan/mhi_wwan_ctrl.c | 2 +- > > > > drivers/net/wwan/rpmsg_wwan_ctrl.c | 2 +- > > > > drivers/net/wwan/t7xx/t7xx_port_wwan.c | 34 +++++++-------- > > > > drivers/net/wwan/wwan_core.c | 59 > > > > ++++++++++++++++++++ > > > > ------ > > > > drivers/net/wwan/wwan_hwsim.c | 2 +- > > > > drivers/usb/class/cdc-wdm.c | 2 +- > > > > include/linux/wwan.h | 6 ++- > > > > 8 files changed, 73 insertions(+), 37 deletions(-) > > > > > > > > diff --git a/drivers/net/wwan/iosm/iosm_ipc_port.c > > > > b/drivers/net/wwan/iosm/iosm_ipc_port.c > > > > index b6d81c627277..dc43b8f0d1af 100644 > > > > --- a/drivers/net/wwan/iosm/iosm_ipc_port.c > > > > +++ b/drivers/net/wwan/iosm/iosm_ipc_port.c > > > > @@ -63,7 +63,8 @@ struct iosm_cdev *ipc_port_init(struct > > > > iosm_imem > > > > *ipc_imem, > > > > ipc_port->ipc_imem = ipc_imem; > > > > > > > > ipc_port->iosm_port = wwan_create_port(ipc_port->dev, > > > > port_type, > > > > - &ipc_wwan_ctrl_o > > > > ps, > > > > ipc_port); > > > > + &ipc_wwan_ctrl_o > > > > ps, 0, > > > > 0, > > > > + ipc_port); > > > > > > How is 0, 0 a valid option here? > > > > > > and if it is a valid option, shouldn't you just have 2 different > > > functions, one that needs these values and one that does > > > not? That > > > would make it more descriptive as to what those options are, and > > > ensure > > > that you get them right. > > > > > > > 0 is a valid option. > > frag_len set to 0 means no split, and headroom set to 0 means no > > reserved headroom in skb. > > > > Sorry, I can't understand why it's more descriptive, could you help > > with more information? It seems to me that the device driver needs > > to > > know what each parameter is and how to set them, and that process > > is > > also required in your proposed solution - "with 2 different > > functions", > > right? > > When you see random integers in the middle of a function call like > this, > you then have to go and look up the function call to determine what > exactly those values are and what is happening. Using 0, 0 as valid > values helps no one out here at all. > > While if the code said: > ipc_port->iosm_port = wwan_create_port(ipc_port->dev, > port_type, > &ipc_wwan_ctrl_ops, > NO_SPLIT, > NO_RESERVED_HEADROOM, > ipc_port); > > > or something like that, it would make more sense, right? > > Remember, we write code for people to read and understand and > maintain > it over time first, for the compiler second. > Yes, you're right, I'll change it: change the random integer to the macro definition to make it more readable, thanks. > thanks, > > greg k-h
Powered by blists - more mailing lists