DSS usage

[Bjørn Mork]

Dmytro Milinevskyy <dmilinevskyy at sequans.com> writes:

> Hi Bjørn,
>
> I've already started to work on a concept of DSS through char dev.
> In the attachment you can find the first trial. 
> It's not extremely stable version but I can exchange data between the host and the device
> even if the main wwan interface is down. The DSS channel is registered as a misc device.
>
> There are several points which so far I don't know how to implement correctly:
> 1. how to publish/delete new DSS channel? Via ioctl for cdc-wdmXX device or via NL msg. Honestly I would prefer to use NL.

Hello Dmytro,

I just got a new idea for you regarding this point.  Don't yet know if
it's a *good* idea, but I thought I could throw it out for you to
consider.  At least it's simple to implement and avoids any new userspace
tool: 

In my attempt to add a "magic" IP session 0 VLAN, I set
NETIF_F_HW_VLAN_CTAG_FILTER in bind and abused the VLAN hardware
filtering infrastructure like this:

static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
{
	struct usbnet *dev = netdev_priv(netdev);
	struct cdc_mbim_state *info = (void *)&dev->data;

	if (vid == IPS0_MAGIC_VLAN)
		info->flags |= FLAG_IPS0_VLAN;

	/* only 256 IP sessions and 256 Device Stream sessions are mapped */
	if (vid >= 512)
		return -EINVAL;
	return 0;
}

static int cdc_mbim_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 vid)
{
	struct usbnet *dev = netdev_priv(netdev);
	struct cdc_mbim_state *info = (void *)&dev->data;

	if (vid == IPS0_MAGIC_VLAN)
		info->flags &= ~FLAG_IPS0_VLAN;
	return 0;
}

static const struct net_device_ops cdc_mbim_netdev_ops = {
	.ndo_open             = usbnet_open,
	.ndo_stop             = usbnet_stop,
	.ndo_start_xmit       = usbnet_start_xmit,
	.ndo_tx_timeout       = usbnet_tx_timeout,
	.ndo_change_mtu       = usbnet_change_mtu,
	.ndo_set_mac_address  = eth_mac_addr,
	.ndo_validate_addr    = eth_validate_addr,
        .ndo_vlan_rx_add_vid  = cdc_mbim_rx_add_vid,
        .ndo_vlan_rx_kill_vid = cdc_mbim_rx_kill_vid,
};


The purpose was to get notifications when VLANs are added and removed,
and I'm really only interested in knowing about the single magic ID.

But what if you use this to automatically create and delete DSS
character devices when one of the DSS VLAN IDs are added or removed?
Just adding something like

       /* create DSS TTY */
       if (vid >= 256 && vid < 512)
                dss_register(info, vid - 256)

       /* remove DSS TTY */
       if (vid >= 256 && vid < 512) {
                struct mbim_dss_channel *dss = dss_find(info, vid - 256)

                if (dss)
                        dss_unregister(info, vid - 256)
        }

should do all you want?

So that would be a simple way to make 'ip' (or any other tool using the
VLAN NL API) create the character device too.  So

  ip link add link wwan0 wwan0.256 type vlan id 256

would now have 2 effects:

  wwan0.256 is created
  /dev/mbim1234-dss0 is created


(BTW, I would strongly suggest using the master netdev name as part of
the tty device, because otherwise you'll have problems mapping these
character devices to multiple MBIM devices).

This would also need some additional magic in the packet delivery, but I
think you'll need that anyway because you cannot just remvoe the
existing API.  You could either just deliver a skb clone to both your
character device and to usbnet_skb_return, or you could select one of
them based on whether the chardev is open or not.

> 2. what is the best way to destroy DSS channels if MBIM device goes offline(i.e. it's being unbound). 
>    I'm afraid that I can't sleep in this context to wait for all DSS users to finish with the cleanup

You'll just have to let the DSS devices live on like zombies, returning
appropriate errors and doing final cleanup in release.  I believe that's
how this is for all character device drivers?

> 3. there's a limitation on the number of misc devices in the system since it uses same major index. 
>    Don't know whether it's a big limitation though.

Probably not, but it would be nice to say "unlimited" :-)

Looking at drivers/char/misc.c it seems that the number of
DYNAMIC_MINORS is as low as 64 by default.  Maybe a misc_device just
isn't the best approach here?

But this is a minor detail which can be changed at any time later.


Bjørn