The concepte looks fine to me, but I have a few comments to the
implementation below.
First: I missed the last part of the discussion around automatic
detection of passthrough mode. Could you give us a short summary of
the
alternatives you tried and why they were dropped?
Hi Bjørn
The other approach was to explicitly check for the rx_handler of the
qmi_wwan
netdev and if it was attached by rmnet driver (rmnet_rx_handler).
If it was indeed attached, then the packet would be queued to stack. I
had
dropped this option since it would mean that atleast the rmnet
rx_handler would
have to be exported and exposed through a new header. This also brings a
tight coupling between qmi_wwan and rmnet driver.
IIUC, userspace will be responsible for doing something like this to
set
up a rmnet map interface:
1) set the qmi_wwan netdev mode to raw-ip using sysfs
2) set the qmi_wwan netdev mode to pass-through using syfs
3) bind an rmnet netdev to the qmi_wwan netdev using netlink
4) configure the device for raw-ip using qmi
5) configure the device for map using qmi
I've just had a quick glance at this, but this function looks like an
almost exact copy of the raw_ip_store(). Why not share that code
instead of copying it?
And while you're at it: There is nothing preventing us from turning on
raw-ip here instead of failing if it is off, us there? I.e. why not
set QMI_WWAN_FLAG_RAWIP when QMI_WWAN_FLAG_PASS_THROUGH is being set,
and clear QMI_WWAN_FLAG_PASS_THROUGH when QMI_WWAN_FLAG_RAWIP is being
cleared?
You're missing the inverse relationship, aren't you? There is nothing
preventing the user from turning off raw-ip again after setting
pass-through.
Do we really need all the notifier stuff here? You don't change the
qmi_wwan netdev since that's already taken care of when setting
QMI_WWAN_FLAG_RAWIP.
AFAICS, QMI_WWAN_FLAG_PASS_THROUGH can be changed without any locking,
notifications or netdev state restrictions. All it does is change the
behaviour on rx, and there is no reason that can't be applied from the
next packet even on a running interface.
We could make pass_through_store just call raw_ip_store (or the part
of it which matters, factored out into a separate function), if
necessary. The rest of pass_through_store just sets or clears the flag.
There is no need to do more than that, is there?
And as noted above, raw_ip_store must also clear
QMI_WWAN_FLAG_PASS_THROUGH if clearing QMI_WWAN_FLAG_RAWIP.
There is no need testing for rawip here, since you enforce that in
pass_through_store().
I can make these changes. With this, steps 1 and 2 are combined.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
