This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Urs Thuermann wrote:
+static int vcan_open(struct net_device *dev)
+{
+ DBG(%s: interface up\n, dev-name);
+
+ netif_start_queue(dev);
+ return 0;
+}
+
+static int vcan_stop(struct net_device *dev)
+{
+ DBG(%s: interface down\n, dev-name);
+
+
Patrick McHardy [EMAIL PROTECTED] writes:
+static int vcan_open(struct net_device *dev)
+static int vcan_stop(struct net_device *dev)
These two functions look unnecessary, there's no need to manage
the queue for pure software devices.
OK, removed.
Please use the NETDEV_TX codes.
OK,
Urs Thuermann wrote:
The new patch is below.
Thanks. I'll look over the other patches later.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Arnaldo Carvalho de Melo [EMAIL PROTECTED] writes:
+#ifdef CONFIG_CAN_DEBUG_DEVICES
+static int debug;
+module_param(debug, int, S_IRUGO);
+#endif
Can debug be a boolean? Like its counterpart on DCCP:
debug used to a bit mask, like it still is in core.h. You can see
this in the test
David Miller wrote:
From: Arnaldo Carvalho de Melo [EMAIL PROTECTED]
Date: Tue, 2 Oct 2007 18:43:25 -0300
I think that helping ctags to find the definition for the debug variable
to see, for instance, if it is a bitmask or a boolean without having to
chose from tons of 'debug' variables is
David Miller wrote:
From: Stephen Hemminger [EMAIL PROTECTED]
Date: Tue, 2 Oct 2007 14:52:36 -0700
Please consider using netif_msg_xxx() and module parameter to set
default message level, like other real network drivers already do.
I keep seeing this recommendation, but the two
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Em Tue, Oct 02, 2007 at 03:10:11PM +0200, Urs Thuermann escreveu:
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs
Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 02, 2007 at 03:10:11PM +0200, Urs Thuermann escreveu:
+
+/* To be moved to linux/can/dev.h */
Is this comment still valid? If so can this move happen now? If not I
think it would be better to stick a FIXME: just before it, no?
Em Tue, Oct 02, 2007 at 05:07:40PM +0200, Oliver Hartkopp escreveu:
Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 02, 2007 at 03:10:11PM +0200, Urs Thuermann escreveu:
+
+/* To be moved to linux/can/dev.h */
Is this comment still valid? If so can this move happen now? If not I
think
Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 02, 2007 at 03:10:11PM +0200, Urs Thuermann escreveu:
+
+#ifdef CONFIG_CAN_DEBUG_DEVICES
+static int debug;
+module_param(debug, int, S_IRUGO);
+#endif
Can debug be a boolean? Like its counterpart on DCCP:
net/dccp/proto.c:
Em Tue, Oct 02, 2007 at 11:02:53PM +0200, Oliver Hartkopp escreveu:
Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 02, 2007 at 03:10:11PM +0200, Urs Thuermann escreveu:
+
+#ifdef CONFIG_CAN_DEBUG_DEVICES
+static int debug;
+module_param(debug, int, S_IRUGO);
+#endif
Can debug be a
From: Arnaldo Carvalho de Melo [EMAIL PROTECTED]
Date: Tue, 2 Oct 2007 18:43:25 -0300
I think that helping ctags to find the definition for the debug variable
to see, for instance, if it is a bitmask or a boolean without having to
chose from tons of 'debug' variables is a good thing.
I
On Tue, 02 Oct 2007 23:02:53 +0200
Oliver Hartkopp [EMAIL PROTECTED] wrote:
Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 02, 2007 at 03:10:11PM +0200, Urs Thuermann escreveu:
+
+#ifdef CONFIG_CAN_DEBUG_DEVICES
+static int debug;
+module_param(debug, int, S_IRUGO);
+#endif
From: Stephen Hemminger [EMAIL PROTECTED]
Date: Tue, 2 Oct 2007 14:52:36 -0700
Please consider using netif_msg_xxx() and module parameter to set
default message level, like other real network drivers already do.
I keep seeing this recommendation, but the two supposedly most mature
and actively
Eric W. Biederman wrote:
Currently IFF_LOOPBACK set in dev-flags means we are dealing
with drivers/net/loopback.c.
This is a very general view, don't you think? The one is an interface
flag and the other one is an interface itself. This looks like a risky
mixture, when there is no clean
Oliver Hartkopp [EMAIL PROTECTED] writes:
Eric W. Biederman wrote:
Currently IFF_LOOPBACK set in dev-flags means we are dealing
with drivers/net/loopback.c.
This is a very general view, don't you think? The one is an interface
flag and the other one is an interface itself. This looks
Eric W. Biederman wrote:
Oliver Hartkopp [EMAIL PROTECTED] writes:
The CAN protocol family is some kind of a closed ecosystem with a
complete different addressing scheme that uses the bare networking
functionality of the Linux Kernel as well as DECNET or ARCNET. You would
never been
Urs Thuermann [EMAIL PROTECTED] writes:
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
I'm trying to wrap my head around the CAN use of IFF_LOOPBACK.
6.2 loopback
As described in
I guess in particular IFF_LOOPBACK means that all packets from
a device will come right back to the current machine, and go
nowhere else.
That usage sounds completely different then the CAN usage which
appears to mean. Broadcast packets will be returned to this machine
as well as being sent out
From: [EMAIL PROTECTED] (Eric W. Biederman)
Date: Thu, 27 Sep 2007 10:16:37 -0600
I guess in particular IFF_LOOPBACK means that all packets from
a device will come right back to the current machine, and go
nowhere else.
That usage sounds completely different then the CAN usage which
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Hello.
In article [EMAIL PROTECTED] (at Tue, 25 Sep 2007 14:20:34 +0200), Urs
Thuermann [EMAIL PROTECTED] says:
Index: net-2.6.24/drivers/net/can/vcan.c
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++
YOSHIFUJI Hideaki / È£ÑÀ [EMAIL PROTECTED] writes:
I'm not a lawyer, but the following lines:
| + * Alternatively, provided that this notice is retained in full, this
~~
| + * software may be distributed under the terms of
Urs Thuermann wrote:
YOSHIFUJI Hideaki / È£ÑÀ [EMAIL PROTECTED] writes:
I'm not a lawyer, but the following lines:
| + * Alternatively, provided that this notice is retained in full, this
~~
| + * software may be distributed
Patrick McHardy wrote:
Urs Thuermann wrote:
YOSHIFUJI Hideaki / È£ÑÀ [EMAIL PROTECTED] writes:
I'm not a lawyer, but the following lines:
| + * Alternatively, provided that this notice is retained in full, this
~~
Oliver Hartkopp wrote:
I do think you need to allow people to select GPLv2 only.
The Linux Kernel is currently under GPLv2 and we just wanted to follow
Linus' mind and so we referenced the COPYING file which many other
source does as well. Indeed it was a hard thing to make our code
Patrick McHardy wrote:
Yoshifuji's point was that the license seems to contradict itself,
it says you may choose GPL, but have to retain BSD. And that is
not about Dual BSD/GPL but about the specific wording.
Got it (finally)!
Trying to specify the GPL Version (and the reference to the
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Urs Thuermann wrote:
Now I think we should consider removing the loopback code from
can_send() and demand from each CAN driver that it *has to* implement
this itself.
I also thought about this solution, which would remove the 'loopback'
parameter in vcan.c and some loopback code in
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
+static int loopback; /* loopback testing. Default: 0 (Off) */
+module_param(loopback, int, S_IRUGO);
+MODULE_PARM_DESC(loopback, Loop back frames (for testing). Default: 0
(Off));
I would still prefer to have this on a
Urs Thuermann wrote:
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
Also looks mostly
Patrick McHardy [EMAIL PROTECTED] writes:
+static int loopback; /* loopback testing. Default: 0 (Off) */
+module_param(loopback, int, S_IRUGO);
+MODULE_PARM_DESC(loopback, Loop back frames (for testing). Default: 0
(Off));
I would still prefer to have this on a per-device level
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Original-Message
Subject: Re: [patch 5/7] CAN: Add virtual CAN netdevice driver
Date: Thu, 12 Jul 2007 00:52:58 +0200
From: Patrick McHardy [EMAIL PROTECTED]
To: Oliver Hartkopp [EMAIL PROTECTED]
Patrick McHardy wrote:
Oliver Hartkopp wrote:
Hi Patrick,
what's your
From: Oliver Hartkopp [EMAIL PROTECTED]
Date: Mon, 16 Jul 2007 08:05:49 +0200
This issue has blocked any ongoing activities for one week now.
Too bad, Patrick is a volunteer just like any other netdev contributor
and you cannot dictate when he or anyone else devotes his or her time
to reviewing
Oliver Hartkopp wrote:
Patrick McHardy wrote:
Still configuration of the network device based on module parameters.
What about people that want loopback and non-loopback devices at the
same time?
the people get the loopback functionality in ANY case. There is indeed
no difference from
Oliver Hartkopp wrote:
This issue has blocked any ongoing activities for one week now.
You also won't be happy if someone raises objections on your code and
then vanishes for discussions for a week (except on other topics on the ML).
Take 2(!) minutes from your valuable time to *understand*
Patrick McHardy wrote:
Oliver Hartkopp wrote:
the people get the loopback functionality in ANY case. There is indeed
no difference from the view of the users, if you change this switch.
The possibility to enable the loopback on vcan driver level is only to ...
1. Test the loopback
Patrick McHardy wrote:
Possible. You could have spent your time working on other obvious
issues, I'm pretty certain running scripts/checkpatch will show
you quite a few.
Ugh! Indeed! Thanks for the hint!! I'll fix this immediately.
Is there anything else that can be checked beforehand?
Hi Patrick,
what's your opinion about my reply to your remark?
Should we just change the module parameter from loopback to
loopbacktest to make the test intention obvious?
Or should we remove the loopback test functionality?
Regards,
Oliver
Oliver Hartkopp wrote:
Patrick McHardy wrote:
Oliver Hartkopp wrote:
Hi Patrick,
what's your opinion about my reply to your remark?
Should we just change the module parameter from loopback to
loopbacktest to make the test intention obvious?
Or should we remove the loopback test functionality?
I'll look into it tommorrow, please be
Patrick McHardy [EMAIL PROTECTED] writes:
I currently still prefer no default interfaces, since we would get rid
of some code.
Me too, and you don't need to worry about compatibility.
Here is a new version of the vcan driver for you to review. We now
use the netlink link creation API
Urs Thuermann wrote:
+ * CAN network devices *should* support a local loopback functionality
+ * (see Documentation/networking/can.txt). To test the handling of CAN
+ * interfaces that do not support the loopback both driver types are
+ * implemented inside this vcan driver. In the case that
Patrick McHardy wrote:
Urs Thuermann wrote:
(..) To test the handling of CAN
+ * interfaces that do not support the loopback both driver types are
+ * implemented inside this vcan driver.
(..)
Still configuration of the network device based on module parameters.
What about people
Patrick McHardy [EMAIL PROTECTED] writes:
It should create as many devices as necessary to operate (similar
to the loopback device) by default. Optional interfaces that are
used for addressing reasons should be manually added by the user
as needed. And it should not use module parameters for
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
It should create as many devices as necessary to operate (similar
to the loopback device) by default. Optional interfaces that are
used for addressing reasons should be manually added by the user
as needed. And it should not use
Patrick McHardy wrote:
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
Is there a reason why you're still doing the allocate n devices
on init thing instead of using the rtnl_link API?
Sorry, it's simply a matter of time. We have been extremely busy with
Oliver Hartkopp wrote:
Patrick McHardy wrote:
Sorry, it's simply a matter of time. We have been extremely busy with
other projects and two presentations (mgmt, customers, and press) the
last two weeks and have worked on the other changes this week. I'm
sorry I haven't yet been able to look
Patrick McHardy wrote:
Oliver Hartkopp wrote:
i was just looking through the mailings regarding your suggested changes
(e.g. in VLAN, DUMMY and IFB) an none of them currently went into the
kernel (..)
They are all in the net-2.6.23 tree.
Ah, ok - that wasn't on my radar as i
Oliver Hartkopp wrote:
@Patrick: The changes in dummy.c and ifb.c for the netlink support do
not look very complicated (not even for me ;-))
I have a patch to make it even simpler, it basically needs only
the rtnl_link_ops structures initialized with one or two members
for devices like dummy
Oliver Hartkopp wrote:
Patrick McHardy wrote:
BTW, in case the loopback device is required for normal
operation it might make sense to create *one* device by
default, but four identical devices seems a bit extreme.
As i wrote before CAN addressing consists of CAN-Identifiers and the
used
Patrick McHardy wrote:
BTW, in case the loopback device is required for normal
operation it might make sense to create *one* device by
default, but four identical devices seems a bit extreme.
As i wrote before CAN addressing consists of CAN-Identifiers and the
used interface. The use of
Urs Thuermann wrote:
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Is there a reason why you're still doing the allocate n devices
on init thing instead of using the rtnl_link API?
-
To
Patrick McHardy [EMAIL PROTECTED] writes:
Is there a reason why you're still doing the allocate n devices
on init thing instead of using the rtnl_link API?
Sorry, it's simply a matter of time. We have been extremely busy with
other projects and two presentations (mgmt, customers, and press)
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
Is there a reason why you're still doing the allocate n devices
on init thing instead of using the rtnl_link API?
Sorry, it's simply a matter of time. We have been extremely busy with
other projects and two presentations
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-Off-By: Oliver Hartkopp [EMAIL PROTECTED]
Signed-Off-By: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
I don't get why you can't directly check the socket option on the
TX path.
We have several types of sockets in the PF_CAN family, two of which
are GPL'ed and which are in the patch series. These are CAN_RAW and
CAN_BCM. The
Oliver Hartkopp wrote:
1. The needed skb-sk is preserved for the rx path (the way 'up').
I was thinking about this, don't CAN frames include the identity
of the sender? That would be the easiest way to determine whether
the frame needs to be delivered.
2. The loopback indication is done by
Patrick McHardy [EMAIL PROTECTED] writes:
I was thinking about this, don't CAN frames include the identity
of the sender? That would be the easiest way to determine whether
the frame needs to be delivered.
No, CAN is quite a broken networking technology (at least in my view,
coming from
Oliver Hartkopp [EMAIL PROTECTED] writes:
2. The loopback indication is done by using the unused skb-protocol in
the tx path.
I don't think we should (mis-)use skb-protocol as a loopback flag.
As the name says, it's the protocol number and not a flag whether
loopback is to be done by the
Urs Thuermann wrote:
Oliver Hartkopp [EMAIL PROTECTED] writes:
2. The loopback indication is done by using the unused skb-protocol in
the tx path.
I don't think we should (mis-)use skb-protocol as a loopback flag.
Yep! After reading the comments from Patrick and Urs i
Oliver Hartkopp wrote:
I think, it goes into the right direction to use skb-pkt_type. The flag
should really be somewhere inside the skb as all back references into
the sk would become sticky in the implementation.
But regarding the use of skb-pkt_type i would suggest to take a closer
look
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
And it allows you have both loopback and non-loopback devices
in case that would be useful.
That sounds very promising. I also was unhappy with the fixed number
of vcan devices at module load time and have thought about
Oliver Hartkopp wrote:
The more i was thinking about my own suggested solution the more it
turned out to be ugly for me ...
Summarizing we have two problems to solve:
1. Identify the originating sk to potentially trash my own sent messages.
2. Indicate a requested local loopback to the
Patrick McHardy [EMAIL PROTECTED] writes:
I have a set of patches coming up that introduce a rtnetlink API
for adding/modifying/deleting software network devices. I would
prefer if you could switch this driver over instead of doing the
create N devices during loading that many current drivers
Patrick McHardy wrote:
Oliver Hartkopp wrote:
Patrick McHardy wrote:
Yes, its working, but only in certain combinations and you're breaking
the rules for skb-cb, making it impossible for other layers to use.
skb-sk is stable at the output path, the regular loopback device
orphans
The more i was thinking about my own suggested solution the more it
turned out to be ugly for me ...
Summarizing we have two problems to solve:
1. Identify the originating sk to potentially trash my own sent messages.
2. Indicate a requested local loopback to the lower (driver) layer.
Regarding
Oliver Hartkopp wrote:
Patrick McHardy wrote:
Urs Thuermann wrote:
+/* tx socket reference pointer: Loopback required if not NULL */
+loop = *(struct sock **)skb-cb != NULL;
Qdiscs might change skb-cb. Maybe use skb-sk?
due to current projects Urs and me had only a
Patrick McHardy wrote:
Urs Thuermann wrote:
+/* tx socket reference pointer: Loopback required if not NULL */
+loop = *(struct sock **)skb-cb != NULL;
Qdiscs might change skb-cb. Maybe use skb-sk?
Hi Patrick,
due to current projects Urs and me had only a short time
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-Off-By: Oliver Hartkopp [EMAIL PROTECTED]
Signed-Off-By: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
Urs Thuermann wrote:
+static int numdev = 4; /* default number of virtual CAN interfaces */
+module_param(numdev, int, S_IRUGO);
+MODULE_PARM_DESC(numdev, Number of virtual CAN devices);
I have a set of patches coming up that introduce a rtnetlink API
for adding/modifying/deleting software
In addition to Patrick's comments.
+
+static void vcan_init(struct net_device *dev)
+{
+ DBG(dev %s\n, dev-name);
+
+ ether_setup(dev);
Do really want to do this?
- you set different flags/type after this.
- do you really want change_mtu to call eth_change_mtu
Better off to
Patrick McHardy wrote:
I have a set of patches coming up that introduce a rtnetlink API
for adding/modifying/deleting software network devices. I would
prefer if you could switch this driver over instead of doing the
create N devices during loading that many current drivers do,
leaving you
Stephen Hemminger wrote:
In addition to Patrick's comments.
+
+static void vcan_init(struct net_device *dev)
+{
+DBG(dev %s\n, dev-name);
+
+ether_setup(dev);
Do really want to do this?
- you set different flags/type after this.
- do you really want change_mtu to
On Wed, 30 May 2007 20:57:33 +0200
Oliver Hartkopp [EMAIL PROTECTED] wrote:
Does 'unneeded' belong to always the last line of the quoted code snippet?
They are all places where you assign zero (or NULL) to something that is
already initialized to zero by alloc_netdev.
--
Stephen
Oliver Hartkopp wrote:
Patrick McHardy wrote:
I have a set of patches coming up that introduce a rtnetlink API
for adding/modifying/deleting software network devices. I would
prefer if you could switch this driver over instead of doing the
create N devices during loading that many current
Stephen Hemminger wrote:
On Wed, 30 May 2007 20:57:33 +0200
Oliver Hartkopp [EMAIL PROTECTED] wrote:
Does 'unneeded' belong to always the last line of the quoted code snippet?
They are all places where you assign zero (or NULL) to something that is
already initialized to zero by
Patrick McHardy wrote:
(..)
Qdiscs might change skb-cb. Maybe use skb-sk?
The loopback functionality in CAN is a bit tricky (maybe you can take a
look into the Documentation patch [7/7] at chapter 3.2 and 4.1.4).
The problem is, that we need a per socket(!) option that enables the
Oliver Hartkopp wrote:
Patrick McHardy wrote:
skb-sk should work here as well since you detect these frames
before queueing to the receiving socket.
Hm - this would indeed be much nicer than using skb-cb.
I think we just had some concerns to use skb-sk for our own
functionality, as
Patrick McHardy wrote:
Oliver Hartkopp wrote:
Btw. it's really worth to look at it again. I just had the idea to
access the loopback flag via skb-sk-opt-loopback (argpfl!) on the TX
path and skb-sk on the RX path. This would avoid skb-sk to be set to
NULL but need to have this loopback
This patch adds the virtual CAN bus (vcan) network driver.
The vcan device is just a loopback device for CAN frames, no
real CAN hardware is involved.
Signed-Off-By: Oliver Hartkopp [EMAIL PROTECTED]
Signed-Off-By: Urs Thuermann [EMAIL PROTECTED]
---
drivers/net/Makefile |1
86 matches
Mail list logo
