Subash Abhinov Kasiviswanathan <subas...@codeaurora.org> writes:
>> thanks, I will test it on Monday.
>>
>> Just a question for my knowledge: is the new sysfs attribute really
>> needed? I mean, is there not any other way to understand from qmi_wwan
>> without user intervention that there is the rmnet device attached?
>>
>> Regards,
>> Daniele
>>
>
> Hi Daniele
>
> You can check for the rx_handler attached to qmi_wwan dev and see if it
> belongs to rmnet. You can use the attached patch for it but it think the
> sysfs way might be a bit cleaner.
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> From f7a2b90948da47ade1b345eddb37b721f5ab65f4 Mon Sep 17 00:00:00 2001
> From: Subash Abhinov Kasiviswanathan <subas...@codeaurora.org>
> Date: Sat, 9 Jun 2018 11:14:22 -0600
> Subject: [PATCH] net: qmi_wwan: Allow packets to pass through to rmnet
>
> Pass through mode is to allow packets in MAP format to be passed
> on to rmnet if the rmnet rx handler is attached to it.
>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subas...@codeaurora.org>
> ---
> drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 4 +++-
> drivers/net/usb/qmi_wwan.c | 10 ++++++++++
> include/linux/if_rmnet.h | 20 ++++++++++++++++++++
> 3 files changed, 33 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/if_rmnet.h
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index 5f4e447..164a18f 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/netlink.h>
> #include <linux/netdevice.h>
> +#include <linux/if_rmnet.h>
> #include "rmnet_config.h"
> #include "rmnet_handlers.h"
> #include "rmnet_vnd.h"
> @@ -48,10 +49,11 @@
> [IFLA_RMNET_FLAGS] = { .len = sizeof(struct ifla_rmnet_flags) },
> };
>
> -static int rmnet_is_real_dev_registered(const struct net_device *real_dev)
> +int rmnet_is_real_dev_registered(const struct net_device *real_dev)
> {
> return rcu_access_pointer(real_dev->rx_handler) == rmnet_rx_handler;
> }
> +EXPORT_SYMBOL(rmnet_is_real_dev_registered);
>
> /* Needs rtnl lock */
> static struct rmnet_port*
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index f52a9be..abdae63 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -22,6 +22,7 @@
> #include <linux/usb/cdc.h>
> #include <linux/usb/usbnet.h>
> #include <linux/usb/cdc-wdm.h>
> +#include <linux/if_rmnet.h>
>
> /* This driver supports wwan (3G/LTE/?) devices using a vendor
> * specific management protocol called Qualcomm MSM Interface (QMI) -
> @@ -354,6 +355,10 @@ static ssize_t add_mux_store(struct device *d, struct
> device_attribute *attr, c
> if (kstrtou8(buf, 0, &mux_id))
> return -EINVAL;
>
> + /* rmnet is already attached here */
> + if (rmnet_is_real_dev_registered(to_net_dev(d)))
> + return -EINVAL;
> +
Maybe rmnet_is_real_dev_registered(dev->net) instead, since we use that
elsewhere in this function?
> /* mux_id [1 - 0x7f] range empirically found */
> if (mux_id < 1 || mux_id > 0x7f)
> return -EINVAL;
> @@ -543,6 +548,11 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct
> sk_buff *skb)
> if (skb->len < dev->net->hard_header_len)
> return 0;
>
> + if (rawip && rmnet_is_real_dev_registered(skb->dev)) {
> + skb->protocol = htons(ETH_P_MAP);
> + return (netif_rx(skb) == NET_RX_SUCCESS);
> + }
Like Daniele said: It would be good to have some way to know when the
rawip condition fails. Or even better: Automatically force rawip mode
when the rmnet driver attaches. But that doesn't seem possible? No
notifications or anything when an rx handler is registered?
Hmm, looking at this I wonder: Is the rawip check really necessary? You
skip all the extra rawip code in the driver anyway, so I don't see how
it matters. But maybe the ethernet header_ops are a problem?
And I wonder about using skb->dev here. Does that really work? I
didn't think we set that until later. Why not use dev->net instead?
Bjørn
