| Message ID | 20210615003016.477-11-ryazanov.s.a@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: WWAN link creation improvements | expand |
On Tue, 2021-06-15 at 03:30 +0300, Sergey Ryazanov wrote: > The WWAN core not only multiplex the netdev configuration data, but > process it too, and needs some space to store its private data > associated with the netdev. Add a structure to keep common WWAN core > data. The structure will be stored inside the netdev private data before > WWAN driver private data and have a field to make it easier to access > the driver data. Also add a helper function that simplifies drivers > access to their data. > > At the moment we use the common WWAN private data to store the WWAN data > link (channel) id at the time the link is created, and report it back to > user using the .fill_info() RTNL callback. This should help the user to > be aware which network interface is binded to which WWAN device data Nit: "binded" -> "bound". > +static size_t wwan_rtnl_get_size(const struct net_device *dev) > +{ > + return > + nla_total_size(4) + /* IFLA_WWAN_LINK_ID */ > + 0; > +} > Not sure I like that code style, but I guess I don't care much either :) johannes
On Tue, Jun 15, 2021 at 10:24 AM Loic Poulain <loic.poulain@linaro.org> wrote: > On Tue, 15 Jun 2021 at 02:30, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: >> The WWAN core not only multiplex the netdev configuration data, but >> process it too, and needs some space to store its private data >> associated with the netdev. Add a structure to keep common WWAN core >> data. The structure will be stored inside the netdev private data before >> WWAN driver private data and have a field to make it easier to access >> the driver data. Also add a helper function that simplifies drivers >> access to their data. > > Would it be possible to store wwan_netdev_priv at the end of priv data instead? > That would allow drivers to use the standard netdev_priv without any change. > And would also simplify forwarding to rmnet (in mhi_net) since rmnet > uses netdev_priv. I do not think that mimicking something by one subsystem for another is generally a good idea. This could look good in a short term, but finally it will become a headache due to involvement of too many entities. IMHO, a suitable approach to share the rmnet library and data structures among drivers is to make the rmnet interface more generic. E.g. consider such netdev/rtnl specific function: static int rmnet_foo_action(struct net_device *dev, ...) { struct rmnet_priv *rmdev = netdev_priv(dev); <do a foo action here> } It could be split into a wrapper and an actual handler: int __rmnet_foo_action(struct rmnet_priv *rmdev, ...) { <do a foo action here> } EXPORT_GPL(__rmnet_foo_action) static int rmnet_foo_action(struct net_device *dev, ...) { struct rmnet_priv *rmdev = netdev_priv(dev); return __rmnet_foo_action(rmdev, ...) } So a call from mhi_net to rmnet could looks like this: static int mhi_net_foo_action(struct net_device *dev, ...) { struct rmnet_priv *rmdev = wwan_netdev_drvpriv(dev); return __rmnet_foo_action(rmdev, ...) } In such a way, only the rmnet users know something special, while other wwan core users and the core itself behave without any surprises. E.g. any regular wwan core minidriver can access the link_id field of the wwan common data by calling netdev_priv() without further calculating the common data offset. >> At the moment we use the common WWAN private data to store the WWAN data >> link (channel) id at the time the link is created, and report it back to >> user using the .fill_info() RTNL callback. This should help the user to >> be aware which network interface is binded to which WWAN device data >> channel. >> >> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> >> CC: M Chetan Kumar <m.chetan.kumar@intel.com> >> CC: Intel Corporation <linuxwwan@intel.com> >> --- >> drivers/net/mhi/net.c | 12 +++++------ >> drivers/net/mhi/proto_mbim.c | 5 +++-- >> drivers/net/wwan/iosm/iosm_ipc_wwan.c | 12 +++++------ >> drivers/net/wwan/wwan_core.c | 29 ++++++++++++++++++++++++++- >> include/linux/wwan.h | 18 +++++++++++++++++ >> 5 files changed, 61 insertions(+), 15 deletions(-) -- Sergey
Hello Johannes, On Tue, Jun 15, 2021 at 10:31 AM Johannes Berg <johannes@sipsolutions.net> wrote: > On Tue, 2021-06-15 at 03:30 +0300, Sergey Ryazanov wrote: >> The WWAN core not only multiplex the netdev configuration data, but >> process it too, and needs some space to store its private data >> associated with the netdev. Add a structure to keep common WWAN core >> data. The structure will be stored inside the netdev private data before >> WWAN driver private data and have a field to make it easier to access >> the driver data. Also add a helper function that simplifies drivers >> access to their data. >> >> At the moment we use the common WWAN private data to store the WWAN data >> link (channel) id at the time the link is created, and report it back to >> user using the .fill_info() RTNL callback. This should help the user to >> be aware which network interface is binded to which WWAN device data > > Nit: "binded" -> "bound". Oh. Will fix it in V2. >> +static size_t wwan_rtnl_get_size(const struct net_device *dev) >> +{ >> + return >> + nla_total_size(4) + /* IFLA_WWAN_LINK_ID */ >> + 0; >> +} >> > > Not sure I like that code style, but I guess I don't care much either :) Yeah. I am not happy with that code style either. But this is the only git-blame friendly style known to me that allows us to add new lines without touching existing ones :( -- Sergey
Hi Sergey, On Sun, 20 Jun 2021 at 16:39, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: > > On Tue, Jun 15, 2021 at 10:24 AM Loic Poulain <loic.poulain@linaro.org> wrote: > > On Tue, 15 Jun 2021 at 02:30, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: > >> The WWAN core not only multiplex the netdev configuration data, but > >> process it too, and needs some space to store its private data > >> associated with the netdev. Add a structure to keep common WWAN core > >> data. The structure will be stored inside the netdev private data before > >> WWAN driver private data and have a field to make it easier to access > >> the driver data. Also add a helper function that simplifies drivers > >> access to their data. > > > > Would it be possible to store wwan_netdev_priv at the end of priv data instead? > > That would allow drivers to use the standard netdev_priv without any change. > > And would also simplify forwarding to rmnet (in mhi_net) since rmnet > > uses netdev_priv. > > I do not think that mimicking something by one subsystem for another > is generally a good idea. This could look good in a short term, but > finally it will become a headache due to involvement of too many > entities. > > IMHO, a suitable approach to share the rmnet library and data > structures among drivers is to make the rmnet interface more generic. > > E.g. consider such netdev/rtnl specific function: > > static int rmnet_foo_action(struct net_device *dev, ...) > { > struct rmnet_priv *rmdev = netdev_priv(dev); > <do a foo action here> > } > > It could be split into a wrapper and an actual handler: > > int __rmnet_foo_action(struct rmnet_priv *rmdev, ...) > { > <do a foo action here> > } > EXPORT_GPL(__rmnet_foo_action) > > static int rmnet_foo_action(struct net_device *dev, ...) > { > struct rmnet_priv *rmdev = netdev_priv(dev); > return __rmnet_foo_action(rmdev, ...) > } > > So a call from mhi_net to rmnet could looks like this: > > static int mhi_net_foo_action(struct net_device *dev, ...) > { > struct rmnet_priv *rmdev = wwan_netdev_drvpriv(dev); > return __rmnet_foo_action(rmdev, ...) > } > > In such a way, only the rmnet users know something special, while > other wwan core users and the core itself behave without any > surprises. E.g. any regular wwan core minidriver can access the > link_id field of the wwan common data by calling netdev_priv() without > further calculating the common data offset. Yes, that would work, but it's an important refactoring since rmnet is all built around the idea that netdev_priv is rmnet_priv, including rx path (netdev_priv(skb->dev)). My initial tests were based on this 'simple' change: https://git.linaro.org/people/loic.poulain/linux.git/commit/?h=wwan_rmnet&id=6308d49790f10615bd33a38d56bc7f101646558f Moreover, a driver like mhi_net also supports non WWAN local link (called mhi_swip), which is a network link between the host and the modem cpu (for modem hosted services...). This link is not managed by the WWAN layer and is directly created by mhi_net. I could create a different driver or set of handlers for this netdev, but it's additional complexity. > >> At the moment we use the common WWAN private data to store the WWAN data > >> link (channel) id at the time the link is created, and report it back to > >> user using the .fill_info() RTNL callback. This should help the user to > >> be aware which network interface is binded to which WWAN device data > >> channel. I wonder if it would not be simpler to store the link ID into netdev->dev_port, it's after all a kind of virtual/logical port. That would only postpone the introduction of a wwan_netdev_priv struct though. Regards, Loic
Hi Loic, On Mon, Jun 21, 2021 at 10:28 AM Loic Poulain <loic.poulain@linaro.org> wrote: > On Sun, 20 Jun 2021 at 16:39, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: >> On Tue, Jun 15, 2021 at 10:24 AM Loic Poulain <loic.poulain@linaro.org> wrote: >>> On Tue, 15 Jun 2021 at 02:30, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: >>>> The WWAN core not only multiplex the netdev configuration data, but >>>> process it too, and needs some space to store its private data >>>> associated with the netdev. Add a structure to keep common WWAN core >>>> data. The structure will be stored inside the netdev private data before >>>> WWAN driver private data and have a field to make it easier to access >>>> the driver data. Also add a helper function that simplifies drivers >>>> access to their data. >>> >>> Would it be possible to store wwan_netdev_priv at the end of priv data instead? >>> That would allow drivers to use the standard netdev_priv without any change. >>> And would also simplify forwarding to rmnet (in mhi_net) since rmnet >>> uses netdev_priv. >> >> I do not think that mimicking something by one subsystem for another >> is generally a good idea. This could look good in a short term, but >> finally it will become a headache due to involvement of too many >> entities. >> >> IMHO, a suitable approach to share the rmnet library and data >> structures among drivers is to make the rmnet interface more generic. >> >> E.g. consider such netdev/rtnl specific function: >> >> static int rmnet_foo_action(struct net_device *dev, ...) >> { >> struct rmnet_priv *rmdev = netdev_priv(dev); >> <do a foo action here> >> } >> >> It could be split into a wrapper and an actual handler: >> >> int __rmnet_foo_action(struct rmnet_priv *rmdev, ...) >> { >> <do a foo action here> >> } >> EXPORT_GPL(__rmnet_foo_action) >> >> static int rmnet_foo_action(struct net_device *dev, ...) >> { >> struct rmnet_priv *rmdev = netdev_priv(dev); >> return __rmnet_foo_action(rmdev, ...) >> } >> >> So a call from mhi_net to rmnet could looks like this: >> >> static int mhi_net_foo_action(struct net_device *dev, ...) >> { >> struct rmnet_priv *rmdev = wwan_netdev_drvpriv(dev); >> return __rmnet_foo_action(rmdev, ...) >> } >> >> In such a way, only the rmnet users know something special, while >> other wwan core users and the core itself behave without any >> surprises. E.g. any regular wwan core minidriver can access the >> link_id field of the wwan common data by calling netdev_priv() without >> further calculating the common data offset. > > Yes, that would work, but it's an important refactoring since rmnet is > all built around the idea that netdev_priv is rmnet_priv, including rx > path (netdev_priv(skb->dev)). > My initial tests were based on this 'simple' change: > https://git.linaro.org/people/loic.poulain/linux.git/commit/?h=wwan_rmnet&id=6308d49790f10615bd33a38d56bc7f101646558f > > Moreover, a driver like mhi_net also supports non WWAN local link > (called mhi_swip), which is a network link between the host and the > modem cpu (for modem hosted services...). This link is not managed by > the WWAN layer and is directly created by mhi_net. I could create a > different driver or set of handlers for this netdev, but it's > additional complexity. Correct me if I am wrong. I just checked the rmnet code and realized that rmnet should work on top of mhi_net and not vice versa. mhi_net should provide some kind of transportation for QMAP packets from a HW device to rmnet. Then rmnet will perform demultiplexing, deaggregation and decapsulation of QMAP packets to pure IP packets. rmnet itself receives these QMAP packets via a network device. So any driver that would like to provide the QMAP transport for rmnet should create a network device for this task. The main issue with the integration of mhi_net with the wwan is that the mhi_net driver should pass its traffic through the rmnet demuxer. While the network device that will be created by the rmnet demuxer will not be a child of a MHI device or a wwan device. So to properly integrate the mhi_net driver with the wwan core netdev capabilities, you should begin to use rmnet not as an independent demux created on top of a transport network interface, but as a library. Am I correctly understanding? Does the same issue appear when we begin a more tight integration of the qmi_wwan USB driver with the wwan core? I would like to say that one way or another, rmnet will be converted to a quite abstract library that should avoid direct access to the network device private data. >>>> At the moment we use the common WWAN private data to store the WWAN data >>>> link (channel) id at the time the link is created, and report it back to >>>> user using the .fill_info() RTNL callback. This should help the user to >>>> be aware which network interface is binded to which WWAN device data >>>> channel. > > I wonder if it would not be simpler to store the link ID into > netdev->dev_port, it's after all a kind of virtual/logical port. > That would only postpone the introduction of a wwan_netdev_priv struct though. I like this idea. This is likely to solve the link id storage problem. But only if we plan to never extend the wwan core private data. Otherwise, as you mention, this only postpones the wwan data structure introduction to a moment when we will need to rework a lot of drivers. Looks like we have no absolutely good solution. Only a set of proposals, each which has its own shortcomings :( -- Sergey
On Mon, 21 Jun 2021 at 19:22, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: > > Hi Loic, > > On Mon, Jun 21, 2021 at 10:28 AM Loic Poulain <loic.poulain@linaro.org> wrote: > > On Sun, 20 Jun 2021 at 16:39, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: > >> On Tue, Jun 15, 2021 at 10:24 AM Loic Poulain <loic.poulain@linaro.org> wrote: > >>> On Tue, 15 Jun 2021 at 02:30, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: > >>>> The WWAN core not only multiplex the netdev configuration data, but > >>>> process it too, and needs some space to store its private data > >>>> associated with the netdev. Add a structure to keep common WWAN core > >>>> data. The structure will be stored inside the netdev private data before > >>>> WWAN driver private data and have a field to make it easier to access > >>>> the driver data. Also add a helper function that simplifies drivers > >>>> access to their data. > >>> > >>> Would it be possible to store wwan_netdev_priv at the end of priv data instead? > >>> That would allow drivers to use the standard netdev_priv without any change. > >>> And would also simplify forwarding to rmnet (in mhi_net) since rmnet > >>> uses netdev_priv. > >> > >> I do not think that mimicking something by one subsystem for another > >> is generally a good idea. This could look good in a short term, but > >> finally it will become a headache due to involvement of too many > >> entities. > >> > >> IMHO, a suitable approach to share the rmnet library and data > >> structures among drivers is to make the rmnet interface more generic. > >> > >> E.g. consider such netdev/rtnl specific function: > >> > >> static int rmnet_foo_action(struct net_device *dev, ...) > >> { > >> struct rmnet_priv *rmdev = netdev_priv(dev); > >> <do a foo action here> > >> } > >> > >> It could be split into a wrapper and an actual handler: > >> > >> int __rmnet_foo_action(struct rmnet_priv *rmdev, ...) > >> { > >> <do a foo action here> > >> } > >> EXPORT_GPL(__rmnet_foo_action) > >> > >> static int rmnet_foo_action(struct net_device *dev, ...) > >> { > >> struct rmnet_priv *rmdev = netdev_priv(dev); > >> return __rmnet_foo_action(rmdev, ...) > >> } > >> > >> So a call from mhi_net to rmnet could looks like this: > >> > >> static int mhi_net_foo_action(struct net_device *dev, ...) > >> { > >> struct rmnet_priv *rmdev = wwan_netdev_drvpriv(dev); > >> return __rmnet_foo_action(rmdev, ...) > >> } > >> > >> In such a way, only the rmnet users know something special, while > >> other wwan core users and the core itself behave without any > >> surprises. E.g. any regular wwan core minidriver can access the > >> link_id field of the wwan common data by calling netdev_priv() without > >> further calculating the common data offset. > > > > Yes, that would work, but it's an important refactoring since rmnet is > > all built around the idea that netdev_priv is rmnet_priv, including rx > > path (netdev_priv(skb->dev)). > > My initial tests were based on this 'simple' change: > > https://git.linaro.org/people/loic.poulain/linux.git/commit/?h=wwan_rmnet&id=6308d49790f10615bd33a38d56bc7f101646558f > > > > Moreover, a driver like mhi_net also supports non WWAN local link > > (called mhi_swip), which is a network link between the host and the > > modem cpu (for modem hosted services...). This link is not managed by > > the WWAN layer and is directly created by mhi_net. I could create a > > different driver or set of handlers for this netdev, but it's > > additional complexity. > > Correct me if I am wrong. I just checked the rmnet code and realized > that rmnet should work on top of mhi_net and not vice versa. mhi_net > should provide some kind of transportation for QMAP packets from a HW > device to rmnet. Then rmnet will perform demultiplexing, deaggregation > and decapsulation of QMAP packets to pure IP packets. Exact mhi_net act as the transport layer in that case. > rmnet itself receives these QMAP packets via a network device. So any > driver that would like to provide the QMAP transport for rmnet should > create a network device for this task. Yes, this is what they call the 'real' device (or lower_dev). > The main issue with the integration of mhi_net with the wwan is that > the mhi_net driver should pass its traffic through the rmnet demuxer. > While the network device that will be created by the rmnet demuxer > will not be a child of a MHI device or a wwan device. So to properly > integrate the mhi_net driver with the wwan core netdev capabilities, > you should begin to use rmnet not as an independent demux created on > top of a transport network interface, but as a library. Am I correctly > understanding? Once the link is created, packets received by the 'real' ndev are automatically forwarded to the upper layer (in that case, rmnet). So the 'transport' netdev doesn't even need to know about the upper layer details. > Does the same issue appear when we begin a more tight integration of > the qmi_wwan USB driver with the wwan core? That should be handled the same way as for mhi_net, indeed. > I would like to say that one way or another, rmnet will be converted > to a quite abstract library that should avoid direct access to the > network device private data. > > >>>> At the moment we use the common WWAN private data to store the WWAN data > >>>> link (channel) id at the time the link is created, and report it back to > >>>> user using the .fill_info() RTNL callback. This should help the user to > >>>> be aware which network interface is binded to which WWAN device data > >>>> channel. > > > > I wonder if it would not be simpler to store the link ID into > > netdev->dev_port, it's after all a kind of virtual/logical port. > > That would only postpone the introduction of a wwan_netdev_priv struct though. > > I like this idea. This is likely to solve the link id storage problem. > But only if we plan to never extend the wwan core private data. > Otherwise, as you mention, this only postpones the wwan data structure > introduction to a moment when we will need to rework a lot of drivers. > > Looks like we have no absolutely good solution. Only a set of > proposals, each which has its own shortcomings :( > > -- Regards, Loic
diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c index 06253acecaa2..cff433d7b984 100644 --- a/drivers/net/mhi/net.c +++ b/drivers/net/mhi/net.c @@ -32,7 +32,7 @@ struct mhi_device_info { static int mhi_ndo_open(struct net_device *ndev) { - struct mhi_net_dev *mhi_netdev = netdev_priv(ndev); + struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev); /* Feed the rx buffer pool */ schedule_delayed_work(&mhi_netdev->rx_refill, 0); @@ -47,7 +47,7 @@ static int mhi_ndo_open(struct net_device *ndev) static int mhi_ndo_stop(struct net_device *ndev) { - struct mhi_net_dev *mhi_netdev = netdev_priv(ndev); + struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev); netif_stop_queue(ndev); netif_carrier_off(ndev); @@ -58,7 +58,7 @@ static int mhi_ndo_stop(struct net_device *ndev) static int mhi_ndo_xmit(struct sk_buff *skb, struct net_device *ndev) { - struct mhi_net_dev *mhi_netdev = netdev_priv(ndev); + struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev); const struct mhi_net_proto *proto = mhi_netdev->proto; struct mhi_device *mdev = mhi_netdev->mdev; int err; @@ -93,7 +93,7 @@ static int mhi_ndo_xmit(struct sk_buff *skb, struct net_device *ndev) static void mhi_ndo_get_stats64(struct net_device *ndev, struct rtnl_link_stats64 *stats) { - struct mhi_net_dev *mhi_netdev = netdev_priv(ndev); + struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev); unsigned int start; do { @@ -322,7 +322,7 @@ static int mhi_net_newlink(void *ctxt, struct net_device *ndev, u32 if_id, if (dev_get_drvdata(&mhi_dev->dev)) return -EBUSY; - mhi_netdev = netdev_priv(ndev); + mhi_netdev = wwan_netdev_drvpriv(ndev); dev_set_drvdata(&mhi_dev->dev, mhi_netdev); mhi_netdev->ndev = ndev; @@ -364,7 +364,7 @@ static int mhi_net_newlink(void *ctxt, struct net_device *ndev, u32 if_id, static void mhi_net_dellink(void *ctxt, struct net_device *ndev, struct list_head *head) { - struct mhi_net_dev *mhi_netdev = netdev_priv(ndev); + struct mhi_net_dev *mhi_netdev = wwan_netdev_drvpriv(ndev); struct mhi_device *mhi_dev = ctxt; unregister_netdevice_queue(ndev, head); diff --git a/drivers/net/mhi/proto_mbim.c b/drivers/net/mhi/proto_mbim.c index fc72b3f6ec9e..bf1ad863237d 100644 --- a/drivers/net/mhi/proto_mbim.c +++ b/drivers/net/mhi/proto_mbim.c @@ -16,6 +16,7 @@ #include <linux/ip.h> #include <linux/mii.h> #include <linux/netdevice.h> +#include <linux/wwan.h> #include <linux/skbuff.h> #include <linux/usb.h> #include <linux/usb/cdc.h> @@ -56,7 +57,7 @@ static void __mbim_errors_inc(struct mhi_net_dev *dev) static int mbim_rx_verify_nth16(struct sk_buff *skb) { - struct mhi_net_dev *dev = netdev_priv(skb->dev); + struct mhi_net_dev *dev = wwan_netdev_drvpriv(skb->dev); struct mbim_context *ctx = dev->proto_data; struct usb_cdc_ncm_nth16 *nth16; int len; @@ -102,7 +103,7 @@ static int mbim_rx_verify_nth16(struct sk_buff *skb) static int mbim_rx_verify_ndp16(struct sk_buff *skb, struct usb_cdc_ncm_ndp16 *ndp16) { - struct mhi_net_dev *dev = netdev_priv(skb->dev); + struct mhi_net_dev *dev = wwan_netdev_drvpriv(skb->dev); int ret; if (le16_to_cpu(ndp16->wLength) < USB_CDC_NCM_NDP16_LENGTH_MIN) { diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c index adb2bd40a404..61ec48468b63 100644 --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c @@ -20,7 +20,7 @@ #define IOSM_IF_ID_PAYLOAD 2 /** - * struct iosm_netdev_priv - netdev private data + * struct iosm_netdev_priv - netdev WWAN driver specific private data * @ipc_wwan: Pointer to iosm_wwan struct * @netdev: Pointer to network interface device structure * @if_id: Interface id for device. @@ -51,7 +51,7 @@ struct iosm_wwan { /* Bring-up the wwan net link */ static int ipc_wwan_link_open(struct net_device *netdev) { - struct iosm_netdev_priv *priv = netdev_priv(netdev); + struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(netdev); struct iosm_wwan *ipc_wwan = priv->ipc_wwan; int if_id = priv->if_id; int ret; @@ -88,7 +88,7 @@ static int ipc_wwan_link_open(struct net_device *netdev) /* Bring-down the wwan net link */ static int ipc_wwan_link_stop(struct net_device *netdev) { - struct iosm_netdev_priv *priv = netdev_priv(netdev); + struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(netdev); netif_stop_queue(netdev); @@ -105,7 +105,7 @@ static int ipc_wwan_link_stop(struct net_device *netdev) static int ipc_wwan_link_transmit(struct sk_buff *skb, struct net_device *netdev) { - struct iosm_netdev_priv *priv = netdev_priv(netdev); + struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(netdev); struct iosm_wwan *ipc_wwan = priv->ipc_wwan; int if_id = priv->if_id; int ret; @@ -178,7 +178,7 @@ static int ipc_wwan_newlink(void *ctxt, struct net_device *dev, if_id >= ARRAY_SIZE(ipc_wwan->sub_netlist)) return -EINVAL; - priv = netdev_priv(dev); + priv = wwan_netdev_drvpriv(dev); priv->if_id = if_id; priv->netdev = dev; priv->ipc_wwan = ipc_wwan; @@ -208,8 +208,8 @@ static int ipc_wwan_newlink(void *ctxt, struct net_device *dev, static void ipc_wwan_dellink(void *ctxt, struct net_device *dev, struct list_head *head) { + struct iosm_netdev_priv *priv = wwan_netdev_drvpriv(dev); struct iosm_wwan *ipc_wwan = ctxt; - struct iosm_netdev_priv *priv = netdev_priv(dev); int if_id = priv->if_id; if (WARN_ON(if_id < IP_MUX_SESSION_START || diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c index b99a737a7d77..3b5545f32c0e 100644 --- a/drivers/net/wwan/wwan_core.c +++ b/drivers/net/wwan/wwan_core.c @@ -807,6 +807,7 @@ static struct net_device *wwan_rtnl_alloc(struct nlattr *tb[], const char *devname = nla_data(tb[IFLA_PARENT_DEV_NAME]); struct wwan_device *wwandev = wwan_dev_get_by_name(devname); struct net_device *dev; + unsigned int priv_size; if (IS_ERR(wwandev)) return ERR_CAST(wwandev); @@ -817,7 +818,8 @@ static struct net_device *wwan_rtnl_alloc(struct nlattr *tb[], goto out; } - dev = alloc_netdev_mqs(wwandev->ops->priv_size, ifname, name_assign_type, + priv_size = sizeof(struct wwan_netdev_priv) + wwandev->ops->priv_size; + dev = alloc_netdev_mqs(priv_size, ifname, name_assign_type, wwandev->ops->setup, num_tx_queues, num_rx_queues); if (dev) { @@ -837,6 +839,7 @@ static int wwan_rtnl_newlink(struct net *src_net, struct net_device *dev, { struct wwan_device *wwandev = wwan_dev_get_by_parent(dev->dev.parent); u32 link_id = nla_get_u32(data[IFLA_WWAN_LINK_ID]); + struct wwan_netdev_priv *priv = netdev_priv(dev); int ret; if (IS_ERR(wwandev)) @@ -848,6 +851,7 @@ static int wwan_rtnl_newlink(struct net *src_net, struct net_device *dev, goto out; } + priv->link_id = link_id; if (wwandev->ops->newlink) ret = wwandev->ops->newlink(wwandev->ops_ctxt, dev, link_id, extack); @@ -881,6 +885,27 @@ static void wwan_rtnl_dellink(struct net_device *dev, struct list_head *head) put_device(&wwandev->dev); } +static size_t wwan_rtnl_get_size(const struct net_device *dev) +{ + return + nla_total_size(4) + /* IFLA_WWAN_LINK_ID */ + 0; +} + +static int wwan_rtnl_fill_info(struct sk_buff *skb, + const struct net_device *dev) +{ + struct wwan_netdev_priv *priv = netdev_priv(dev); + + if (nla_put_u32(skb, IFLA_WWAN_LINK_ID, priv->link_id)) + goto nla_put_failure; + + return 0; + +nla_put_failure: + return -EMSGSIZE; +} + static const struct nla_policy wwan_rtnl_policy[IFLA_WWAN_MAX + 1] = { [IFLA_WWAN_LINK_ID] = { .type = NLA_U32 }, }; @@ -892,6 +917,8 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = { .validate = wwan_rtnl_validate, .newlink = wwan_rtnl_newlink, .dellink = wwan_rtnl_dellink, + .get_size = wwan_rtnl_get_size, + .fill_info = wwan_rtnl_fill_info, .policy = wwan_rtnl_policy, }; diff --git a/include/linux/wwan.h b/include/linux/wwan.h index 14c9a19f3bf0..37a14af95845 100644 --- a/include/linux/wwan.h +++ b/include/linux/wwan.h @@ -8,6 +8,7 @@ #include <linux/kernel.h> #include <linux/skbuff.h> #include <linux/netlink.h> +#include <linux/netdevice.h> /** * enum wwan_port_type - WWAN port types @@ -117,6 +118,23 @@ void wwan_port_txon(struct wwan_port *port); */ void *wwan_port_get_drvdata(struct wwan_port *port); +/** + * struct wwan_netdev_priv - WWAN core network device private data + * @link_id: WWAN device data link id + * @drv_priv: driver private data area, size is determined in &wwan_ops + */ +struct wwan_netdev_priv { + u32 link_id; + + /* must be last */ + u8 drv_priv[] __aligned(sizeof(void *)); +}; + +static inline void *wwan_netdev_drvpriv(struct net_device *dev) +{ + return ((struct wwan_netdev_priv *)netdev_priv(dev))->drv_priv; +} + /** * Used to indicate that the WWAN core should not create a default network * link.
The WWAN core not only multiplex the netdev configuration data, but process it too, and needs some space to store its private data associated with the netdev. Add a structure to keep common WWAN core data. The structure will be stored inside the netdev private data before WWAN driver private data and have a field to make it easier to access the driver data. Also add a helper function that simplifies drivers access to their data. At the moment we use the common WWAN private data to store the WWAN data link (channel) id at the time the link is created, and report it back to user using the .fill_info() RTNL callback. This should help the user to be aware which network interface is binded to which WWAN device data channel. Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> CC: M Chetan Kumar <m.chetan.kumar@intel.com> CC: Intel Corporation <linuxwwan@intel.com> --- drivers/net/mhi/net.c | 12 +++++------ drivers/net/mhi/proto_mbim.c | 5 +++-- drivers/net/wwan/iosm/iosm_ipc_wwan.c | 12 +++++------ drivers/net/wwan/wwan_core.c | 29 ++++++++++++++++++++++++++- include/linux/wwan.h | 18 +++++++++++++++++ 5 files changed, 61 insertions(+), 15 deletions(-)