Subject: Re: [PATCH net-next 1/1 v2] net: rmnet_data: Initial implementation

From: Subash Abhinov Kasiviswanathan <[email-masked]>
Date: Fri, 24 Mar 2017 18:49:17 -0600

> (re-sending from an address that's actually subscribed to netdev@...)
> 
> The first thing that jumps out at me is "why isn't this using
> rtnl_link_ops?"
> 
> To me (and perhaps I'm completely wrong) the structure here is a lot
> like VLAN interfaces.  You have a base device (whether that's a netdev
> or not) and you essentially "ip link add link cdc-wdm0 name rmnet0 type
> rmnet id 5".  Does the aggregation happen only on the downlink (eg,
> device -> host) or can the host send aggregated packets too?

Hi Dan

Yes, you are correct. We associate this driver with a physical device 
and
then create rmnet devices over it as needed for multiplexing.

Aggregation is supported both on downlink and uplink by Qualcomm
Technologies, Inc. modem hardware. This initial patchset implements only
downlink aggregation since uplink aggregation is rarely enabled or used.
I'll send a separate patchset for that.

> Using rtnl_link_ops would get rid of ASSOC_NET_DEV, UNASSOC_NET_DEV,
> NEW_VND, NEW_VND_WITH_PREFIX, and FREE_VND.  GET_NET_DEV_ASSOC goes
> away becuase you use normal 'kobject' associations and you can derive
> the rmnet parent through sysfs links.  rmnet_nl_msg_s goes away,
> because you can use nla_policy.
> 
> Just a thought; there seems to be a ton of overlap with rtnl_link_ops
> in the control plane here.

As of now, we have been using a custom netlink userspace tool for
configuring the interfaces by listening to RMNET_NETLINK_PROTO events.
Does that mean that configuration needs to be moved to ip tool by
adding a new option for rmnet (like how you have mentioned above)
and add additional options for end point configuration as well?

> Any thoughts on how this plays with net namespaces?

We have not tried it with different net namespaces since we never had
such use cases internally. We can look into this.

> Also, I'm not sure if it make sense to provide first class tracepoints
> for a specific driver, as it's not clear if they are userspace API or
> not and thus may need to be kept stable.   Or are perf probes enough
> instead?

We have some tracepoints which are in datapath and some for specific
events such as device unregistration and configuration events 
association.
Most of these devices are on ARM64 and need to support older kernels,
(from 3.10) so we had to rely on tracepoints. I believe ARM64 got trace
point support somewhat recently. I was not aware of the restriction of
tracepoints, so I can remove that.

> 
> What's RMNET_EPMODE_BRIDGE and how is it used?

RMNET_EPMODE_BRIDGE is for bridging two different physical interfaces.
An example is sending raw bytes from hardware to USB - this can be used
if a PC connected by USB would require the data in the MAP format.