* Re: j1939
From: Marc Kleine-Budde @ 2016-10-04 17:32 UTC (permalink / raw)
To: David Jander, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <20161004135701.GA25008@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 745 bytes --]
On 10/04/2016 03:57 PM, Kurt Van Dijck wrote:
> Recently, someone on this list was confronted with an engine ECU that
> dropped TP that had the last packet not 8 bytes long.
> And I believe I've encountered an issue a long time ago where this
> happened on all PGNs.
> So I introduced this option, having 3 possibilities:
> * no padding
> * padding for TP
> * padding for all PGN.
How to switch between padding for TP only and padding for all PGN?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-04 17:31 UTC (permalink / raw)
To: David Jander; +Cc: Austin Schuh, Oliver Hartkopp, Marc Kleine-Budde, linux-can
In-Reply-To: <20161004164725.08802a61@erd980>
>
> > But often, especially for proprietary messages, not all 8 bytes are
> > used, and the DLC is cut accordingly.
> > And less often, this is done for TP too.
> > I also did that for as long as I can remember.
> > As long as all defined bytes are there, this should not really be a
> > problem.
> >
> > Recently, someone on this list was confronted with an engine ECU that
> > dropped TP that had the last packet not 8 bytes long.
> > And I believe I've encountered an issue a long time ago where this
> > happened on all PGNs.
> > So I introduced this option, having 3 possibilities:
> > * no padding
> > * padding for TP
> > * padding for all PGN.
>
> I have yet to check, but I fear that in ISOBus conformance tests on some
> places at least this is tested for. I'd say that if the standard says "pad and
> fill with 0xff", any ECU should do this and not doing it should be regarded as
> a bug...
I (EIA Electronics) got a certified ISOBus Terminal with this code, back
in 2011, with this code (well, the code back then). I cannot remember
that I needed to pad TP.
The networking layer was the easy part in Potsdam, the layers above were
more difficult :-)
Things may have become more strict in between.
And we should be prepared for the strict setting.
On the other hand, during debugging, not padding helps identifying the
real data, and this has been valueable too for me.
I think we agree on what must be possible, we may just differ on the
defaults and how to set things.
Kurt
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-04 17:51 UTC (permalink / raw)
To: David Jander, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <20161004173129.GB25008@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 2130 bytes --]
On 10/04/2016 07:31 PM, Kurt Van Dijck wrote:
>>
>>> But often, especially for proprietary messages, not all 8 bytes are
>>> used, and the DLC is cut accordingly.
>>> And less often, this is done for TP too.
>>> I also did that for as long as I can remember.
>>> As long as all defined bytes are there, this should not really be a
>>> problem.
>>>
>>> Recently, someone on this list was confronted with an engine ECU that
>>> dropped TP that had the last packet not 8 bytes long.
>>> And I believe I've encountered an issue a long time ago where this
>>> happened on all PGNs.
>>> So I introduced this option, having 3 possibilities:
>>> * no padding
>>> * padding for TP
>>> * padding for all PGN.
>>
>> I have yet to check, but I fear that in ISOBus conformance tests on some
>> places at least this is tested for. I'd say that if the standard says "pad and
>> fill with 0xff", any ECU should do this and not doing it should be regarded as
>> a bug...
>
> I (EIA Electronics) got a certified ISOBus Terminal with this code, back
> in 2011, with this code (well, the code back then). I cannot remember
> that I needed to pad TP.
> The networking layer was the easy part in Potsdam, the layers above were
> more difficult :-)
>
> Things may have become more strict in between.
> And we should be prepared for the strict setting.
> On the other hand, during debugging, not padding helps identifying the
> real data, and this has been valueable too for me.
>
> I think we agree on what must be possible, we may just differ on the
> defaults and how to set things.
It seems, or rather what I'd like to see is that we want most things to
be per socket option. All module parameters should go away. The default
for transport_max_size might be a system wide value, similar to
/proc/sys/net/core/rmem_max.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Patrick Menschel @ 2016-10-04 17:54 UTC (permalink / raw)
To: Marc Kleine-Budde, David Jander, Austin Schuh, Oliver Hartkopp,
linux-can
In-Reply-To: <8972643b-d6c8-1880-3360-4f8ef129cc4f@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]
>> Recently, someone on this list was confronted with an engine ECU that
>> dropped TP that had the last packet not 8 bytes long.
>> And I believe I've encountered an issue a long time ago where this
>> happened on all PGNs.
>> So I introduced this option, having 3 possibilities:
>> * no padding
>> * padding for TP
>> * padding for all PGN.
> How to switch between padding for TP only and padding for all PGN?
>
> Marc
>
I'm working with J1939 regularly and programmed J1939 in Python3 a while
ago.
From my experience, message construction is responsibility of the user
program.
There are numerous SPNs to be implemented in a message while
unimplemented bits are to be padded with ones.
It may be possible to pad all messages with 0xFF but that doesn't solve
the non implemented bits in front of the padding.
I think it's best practice to fulfill the requirements by creating a
j1939 message with 8 Bytes of 0xFF and then replace the bits for a
specific SPN.
This would be in user space when calling the J1939 message constructor.
BTW many j1939 descriptor files types such as the PCAN ".sym" files,
define SPNs as a bit offset and a length (additionally with type, factor
and offset as compu_method), so no need to invent the wheel again.
Regards,
Patrick
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3709 bytes --]
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-04 18:38 UTC (permalink / raw)
To: Patrick Menschel
Cc: Marc Kleine-Budde, David Jander, Austin Schuh, Oliver Hartkopp,
linux-can
In-Reply-To: <7661b770-574f-6337-d9a9-ff2103ebb8a6@posteo.de>
>
> >> Recently, someone on this list was confronted with an engine ECU that
> >> dropped TP that had the last packet not 8 bytes long.
> >> And I believe I've encountered an issue a long time ago where this
> >> happened on all PGNs.
> >> So I introduced this option, having 3 possibilities:
> >> * no padding
> >> * padding for TP
> >> * padding for all PGN.
> > How to switch between padding for TP only and padding for all PGN?
0: no padding
1: TP padding
2: all PGN padding
> >
> > Marc
> >
>
> I'm working with J1939 regularly and programmed J1939 in Python3 a while
> ago.
>
> From my experience, message construction is responsibility of the user
> program.
>
> There are numerous SPNs to be implemented in a message while
> unimplemented bits are to be padded with ones.
> It may be possible to pad all messages with 0xFF but that doesn't solve
> the non implemented bits in front of the padding.
>
> I think it's best practice to fulfill the requirements by creating a
> j1939 message with 8 Bytes of 0xFF and then replace the bits for a
> specific SPN.
> This would be in user space when calling the J1939 message constructor.
Although this does not apply to TP, it does apply to all other PGN's.
So there is no real need to pad all PGN's.
Kurt
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-04 18:40 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: David Jander, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <8f5ebcf8-8bb6-a9f0-33af-47d4a551e811@pengutronix.de>
> On 10/04/2016 07:31 PM, Kurt Van Dijck wrote:
> >>
> >>> But often, especially for proprietary messages, not all 8 bytes are
> >>> used, and the DLC is cut accordingly.
> >>> And less often, this is done for TP too.
> >>> I also did that for as long as I can remember.
> >>> As long as all defined bytes are there, this should not really be a
> >>> problem.
> >>>
> >>> Recently, someone on this list was confronted with an engine ECU that
> >>> dropped TP that had the last packet not 8 bytes long.
> >>> And I believe I've encountered an issue a long time ago where this
> >>> happened on all PGNs.
> >>> So I introduced this option, having 3 possibilities:
> >>> * no padding
> >>> * padding for TP
> >>> * padding for all PGN.
> >>
> >> I have yet to check, but I fear that in ISOBus conformance tests on some
> >> places at least this is tested for. I'd say that if the standard says "pad and
> >> fill with 0xff", any ECU should do this and not doing it should be regarded as
> >> a bug...
> >
> > I (EIA Electronics) got a certified ISOBus Terminal with this code, back
> > in 2011, with this code (well, the code back then). I cannot remember
> > that I needed to pad TP.
> > The networking layer was the easy part in Potsdam, the layers above were
> > more difficult :-)
> >
> > Things may have become more strict in between.
> > And we should be prepared for the strict setting.
> > On the other hand, during debugging, not padding helps identifying the
> > real data, and this has been valueable too for me.
> >
> > I think we agree on what must be possible, we may just differ on the
> > defaults and how to set things.
>
> It seems, or rather what I'd like to see is that we want most things to
> be per socket option. All module parameters should go away. The default
> for transport_max_size might be a system wide value, similar to
> /proc/sys/net/core/rmem_max.
do you propose to add /proc files?
I'm willing to replace module parameters with /proc files, but I don't
see the real advantage yet. And module parameters were quicker :-)
Kurt
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-04 18:43 UTC (permalink / raw)
To: Patrick Menschel, David Jander, Austin Schuh, Oliver Hartkopp,
linux-can
In-Reply-To: <20161004183848.GC25008@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 1171 bytes --]
On 10/04/2016 08:38 PM, Kurt Van Dijck wrote:
>
>>
>>>> Recently, someone on this list was confronted with an engine ECU that
>>>> dropped TP that had the last packet not 8 bytes long.
>>>> And I believe I've encountered an issue a long time ago where this
>>>> happened on all PGNs.
>>>> So I introduced this option, having 3 possibilities:
>>>> * no padding
>>>> * padding for TP
>>>> * padding for all PGN.
>>> How to switch between padding for TP only and padding for all PGN?
>
> 0: no padding
> 1: TP padding
> 2: all PGN padding
Hmmm, have I taken the latest and greatest code of yours?
"padding" is static and only used once in main.c
https://git.kernel.org/cgit/linux/kernel/git/mkl/linux-can-next.git/tree/net/can/j1939/main.c?h=j1939#n45
https://git.kernel.org/cgit/linux/kernel/git/mkl/linux-can-next.git/tree/net/can/j1939/main.c?h=j1939#n171
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-04 18:46 UTC (permalink / raw)
To: David Jander, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <20161004184036.GD25008@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 1023 bytes --]
On 10/04/2016 08:40 PM, Kurt Van Dijck wrote:
>>> I think we agree on what must be possible, we may just differ on the
>>> defaults and how to set things.
>>
>> It seems, or rather what I'd like to see is that we want most things to
>> be per socket option. All module parameters should go away. The default
>> for transport_max_size might be a system wide value, similar to
>> /proc/sys/net/core/rmem_max.
>
> do you propose to add /proc files?
No - However I'm not sure yet, though.
> I'm willing to replace module parameters with /proc files, but I don't
> see the real advantage yet. And module parameters were quicker :-)
When trying to bring new code into the kernel, this is probably not the
best argument :)
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-05 5:08 UTC (permalink / raw)
To: Marc Kleine-Budde, Patrick Menschel, David Jander, Austin Schuh,
Oliver Hartkopp, linux-can
In-Reply-To: <a4af635e-9a0c-d5df-5a90-4722967f0ac1@pengutronix.de>
On 4 October 2016 20:43:01 CEST, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>On 10/04/2016 08:38 PM, Kurt Van Dijck wrote:
>>
>>>
>>>>> Recently, someone on this list was confronted with an engine ECU
>that
>>>>> dropped TP that had the last packet not 8 bytes long.
>>>>> And I believe I've encountered an issue a long time ago where this
>>>>> happened on all PGNs.
>>>>> So I introduced this option, having 3 possibilities:
>>>>> * no padding
>>>>> * padding for TP
>>>>> * padding for all PGN.
>>>> How to switch between padding for TP only and padding for all PGN?
>>
>> 0: no padding
>> 1: TP padding
>> 2: all PGN padding
>
>Hmmm, have I taken the latest and greatest code of yours?
>
>"padding" is static and only used once in main.c
>
>https://git.kernel.org/cgit/linux/kernel/git/mkl/linux-can-next.git/tree/net/can/j1939/main.c?h=j1939#n45
>
>https://git.kernel.org/cgit/linux/kernel/git/mkl/linux-can-next.git/tree/net/can/j1939/main.c?h=j1939#n171
>
I see.
I wii check later. Maybe I still needed to extend ... In that case, consider the above a configuration proposal?
Kurt
>Marc
Sent from a small mobile device
^ permalink raw reply
* Re: j1939
From: David Jander @ 2016-10-05 6:48 UTC (permalink / raw)
To: Patrick Menschel
Cc: Marc Kleine-Budde, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <7661b770-574f-6337-d9a9-ff2103ebb8a6@posteo.de>
On Tue, 4 Oct 2016 19:54:30 +0200
Patrick Menschel <menschel.p@posteo.de> wrote:
> >> Recently, someone on this list was confronted with an engine ECU that
> >> dropped TP that had the last packet not 8 bytes long.
> >> And I believe I've encountered an issue a long time ago where this
> >> happened on all PGNs.
> >> So I introduced this option, having 3 possibilities:
> >> * no padding
> >> * padding for TP
> >> * padding for all PGN.
> > How to switch between padding for TP only and padding for all PGN?
> >
> > Marc
> >
>
> I'm working with J1939 regularly and programmed J1939 in Python3 a while
> ago.
>
> From my experience, message construction is responsibility of the user
> program.
I agree.
> There are numerous SPNs to be implemented in a message while
> unimplemented bits are to be padded with ones.
> It may be possible to pad all messages with 0xFF but that doesn't solve
> the non implemented bits in front of the padding.
I think the kernel should never modify packets from user-space in such a way.
I think we should limit this option to chose between:
0: (default) Strictly standards compliant (i.e. pad TP messages) and
1: Not pad TP messages (may break standard).
> I think it's best practice to fulfill the requirements by creating a
> j1939 message with 8 Bytes of 0xFF and then replace the bits for a
> specific SPN.
That may be true for many standard PGN's, but not for all, and certainly not
for proprietary messages. Again, IMHO, the kernel should not mess with
user-space messages.
> This would be in user space when calling the J1939 message constructor.
Precisely.
> BTW many j1939 descriptor files types such as the PCAN ".sym" files,
> define SPNs as a bit offset and a length (additionally with type, factor
> and offset as compu_method), so no need to invent the wheel again.
Don't know exactly what you mean, but this sounds user-space specific to me.
> Regards,
> Patrick
>
Best regards,
--
David Jander
Protonic Holland.
^ permalink raw reply
* Re: j1939
From: David Jander @ 2016-10-05 6:50 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <8972643b-d6c8-1880-3360-4f8ef129cc4f@pengutronix.de>
On Tue, 4 Oct 2016 19:32:02 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 10/04/2016 03:57 PM, Kurt Van Dijck wrote:
> > Recently, someone on this list was confronted with an engine ECU that
> > dropped TP that had the last packet not 8 bytes long.
> > And I believe I've encountered an issue a long time ago where this
> > happened on all PGNs.
> > So I introduced this option, having 3 possibilities:
> > * no padding
> > * padding for TP
> > * padding for all PGN.
>
> How to switch between padding for TP only and padding for all PGN?
Please don't. I think we should get rid of that last option. It is like trying
to correct HTTP GET request syntax in kernel-space if user-space is too lazy
to get it right. Don't do that.
Best regards,
--
David Jander
Protonic Holland.
^ permalink raw reply
* Re: [PATCH v2 00/12] can: rx-offload: add implmentation and switch flexcan driver to use it
From: Alexander Stein @ 2016-10-05 12:37 UTC (permalink / raw)
To: David Jander
Cc: Marc Kleine-Budde, linux-can, Martin Däumler,
Daniel Krüger
In-Reply-To: <20161004143315.23423e56@erd980>
Hi David,
On Tuesday 04 October 2016 14:33:15, David Jander wrote:
> > In summary the non-RT case seems fine now, but I wonder what causes the
> > delays on RT to the CAN-IRQ-Thread which seem to be about 500us (bus time
> > of 6 4Byte CAN frames).
>
> Thanks for testing!
> Well, in -RT I guess anything with a higher priority than the CAN-IRQ can
> cause a delay... but since going from -rt13 to -rt14 improved the situation,
> this sounds like bugs in the -rt code. Are you running any code in
> SCHED_FIFO that you don't run in the non-RT case?
AFAIK no userspace process is put into SCHED_FIFO and only the priority of
CAN-IRQ-Thread is increased to 91. Userspace runs with unchanged priority. So
yes I guess there is still problem in -rt code.
> It's nevertheless very nice to see that for non-RT, you got from losing
> messages to not losing any message with this patchset, even on a lowly
> i.MX35 at 1Mbit/s! This proves that the patchset is certainly worth it.
I would expect the same from running with RT enabled, especially as the CAN-
IRQ-Thread has SCHED_FIFO wuth priority 91. There shouldn't be much code which
can delay that thread.
Best regards,
Alexander
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-05 18:02 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: David Jander, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <48bf45a3-b724-a546-62ee-31a27dc3db33@pengutronix.de>
> On 10/04/2016 08:40 PM, Kurt Van Dijck wrote:
> >>> I think we agree on what must be possible, we may just differ on the
> >>> defaults and how to set things.
> >>
> >> It seems, or rather what I'd like to see is that we want most things to
> >> be per socket option. All module parameters should go away. The default
> >> for transport_max_size might be a system wide value, similar to
> >> /proc/sys/net/core/rmem_max.
> >
> > do you propose to add /proc files?
In that case, I didn't understand your point :-(
>
> No - However I'm not sure yet, though.
>
> > I'm willing to replace module parameters with /proc files, but I don't
> > see the real advantage yet. And module parameters were quicker :-)
>
> When trying to bring new code into the kernel, this is probably not the
> best argument :)
Well, in my experience, the infrastructures are built so that
easy-to-read code do the right thing in the right way.
In that regard, I suspect module parameters may be the preferred way?
Kurt
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-06 7:26 UTC (permalink / raw)
To: David Jander, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <20161005180205.GE25008@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 1871 bytes --]
On 10/05/2016 08:02 PM, Kurt Van Dijck wrote:
>> On 10/04/2016 08:40 PM, Kurt Van Dijck wrote:
>>>>> I think we agree on what must be possible, we may just differ on the
>>>>> defaults and how to set things.
>>>>
>>>> It seems, or rather what I'd like to see is that we want most things to
>>>> be per socket option. All module parameters should go away. The default
>>>> for transport_max_size might be a system wide value, similar to
>>>> /proc/sys/net/core/rmem_max.
>>>
>>> do you propose to add /proc files?
>
>> No - However I'm not sure yet, though.
> In that case, I didn't understand your point :-(
The rmem_max and rmem_default and their wmem friends define both a
system wide default and a maximum for the read and write buffer of
sockets. And there are two sockopts to change the default up to the
maximum value that correspondes to the _max value. Where to put this
value is another question. As it's a system wide setting /proc might be
a sensible place.
>>> I'm willing to replace module parameters with /proc files, but I don't
>>> see the real advantage yet. And module parameters were quicker :-)
>>
>> When trying to bring new code into the kernel, this is probably not the
>> best argument :)
>
> Well, in my experience, the infrastructures are built so that
> easy-to-read code do the right thing in the right way.
> In that regard, I suspect module parameters may be the preferred way?
Some weeks ago Grek turned down a patch, as it introduced new module
parameters and said quite clearly no new module parameters.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-06 7:41 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: David Jander, Austin Schuh, Oliver Hartkopp, linux-can
In-Reply-To: <5fb6231c-4461-2be7-bd47-530aa62c121f@pengutronix.de>
--- Original message ---
> Date: Thu, 6 Oct 2016 09:26:00 +0200
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> To: David Jander <david@protonic.nl>, Austin Schuh
> <austin@peloton-tech.com>, Oliver Hartkopp <socketcan@hartkopp.net>,
> linux-can <linux-can@vger.kernel.org>
> Subject: Re: j1939
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
> Icedove/45.2.0
>
> On 10/05/2016 08:02 PM, Kurt Van Dijck wrote:
> >> On 10/04/2016 08:40 PM, Kurt Van Dijck wrote:
> >>>>> I think we agree on what must be possible, we may just differ on the
> >>>>> defaults and how to set things.
> >>>>
> >>>> It seems, or rather what I'd like to see is that we want most things to
> >>>> be per socket option. All module parameters should go away. The default
> >>>> for transport_max_size might be a system wide value, similar to
> >>>> /proc/sys/net/core/rmem_max.
> >>>
> >>> do you propose to add /proc files?
> >
> >> No - However I'm not sure yet, though.
> > In that case, I didn't understand your point :-(
>
> The rmem_max and rmem_default and their wmem friends define both a
> system wide default and a maximum for the read and write buffer of
> sockets. And there are two sockopts to change the default up to the
> maximum value that correspondes to the _max value. Where to put this
> value is another question. As it's a system wide setting /proc might be
> a sensible place.
I see.
Your example happened to live in proc, but that wasn't the point.
I agree with something like that, although I only envisioned xxx_default
and not xxx_max.
>
> >>> I'm willing to replace module parameters with /proc files, but I don't
> >>> see the real advantage yet. And module parameters were quicker :-)
> >>
> >> When trying to bring new code into the kernel, this is probably not the
> >> best argument :)
> >
> > Well, in my experience, the infrastructures are built so that
> > easy-to-read code do the right thing in the right way.
> > In that regard, I suspect module parameters may be the preferred way?
>
> Some weeks ago Grek turned down a patch, as it introduced new module
> parameters and said quite clearly no new module parameters.
>
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-06 8:44 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, David Jander, Kurt Van Dijck
In-Reply-To: <a6ad3606-b295-dc72-c557-098c45769cb9@pengutronix.de>
Hey Marc,
I was able to take a close look in the code.
> Hello,
>
> I've ported Kurt's latest j1939 patches to linux-4.7. I've squashed all
> patches into one. Then a patch to undo unused modifications and a bunch
> of patches to make checkpatch happy.
>
> I've pushed the result to
>
> https://git.kernel.org/cgit/linux/kernel/git/mkl/linux-can-next.git/log/?h=j1939
>
> The remaining warning is about a case statement without fall through
> annotations and/or breaks:
>
I rebased this code for my own use so that I have access to my
individual commits. You will find this on
git://github.com/kurt-vd/linux, branch j1939d-v4.7
I pushed 2 extra commits:
* 3dfa139: Pad TP data packets only
This is based on the discussions here. I promised to move the
padding...
* e2447ef: make TP data transmission work again
This commit fixes a mistake you've done with the fall-trough
statements in j1939tp_txnext (this isn't the easiest state machien on
Earth :-) ).
Kind regards,
Kurt
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-06 8:59 UTC (permalink / raw)
To: linux-can, David Jander, Kurt Van Dijck
In-Reply-To: <20161006084407.GB11907@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 1216 bytes --]
On 10/06/2016 10:44 AM, Kurt Van Dijck wrote:
>> The remaining warning is about a case statement without fall through
>> annotations and/or breaks:
>>
> I rebased this code for my own use so that I have access to my
> individual commits. You will find this on
>
> git://github.com/kurt-vd/linux, branch j1939d-v4.7
As long as the diff between you and my branch is zero :)
> I pushed 2 extra commits:
> * 3dfa139: Pad TP data packets only
> This is based on the discussions here. I promised to move the
> padding...
Thanks.
> * e2447ef: make TP data transmission work again
> This commit fixes a mistake you've done with the fall-trough
> statements in j1939tp_txnext (this isn't the easiest state machien on
> Earth :-) ).
Thanks.
I have cherry picked both and will push them to v4.7. If everything
works on the v4.7 branch, I'll squash most of it together an move to v4.8.
Thoughts?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-06 9:24 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, David Jander, Kurt Van Dijck
In-Reply-To: <a6920929-30e0-0c69-52f1-96998698bbf6@pengutronix.de>
>
> On 10/06/2016 10:44 AM, Kurt Van Dijck wrote:
> >> The remaining warning is about a case statement without fall through
> >> annotations and/or breaks:
> >>
> > I rebased this code for my own use so that I have access to my
> > individual commits. You will find this on
> >
> > git://github.com/kurt-vd/linux, branch j1939d-v4.7
>
> As long as the diff between you and my branch is zero :)
That is the case.
>
> > I pushed 2 extra commits:
> > * 3dfa139: Pad TP data packets only
> > This is based on the discussions here. I promised to move the
> > padding...
>
> Thanks.
>
> > * e2447ef: make TP data transmission work again
> > This commit fixes a mistake you've done with the fall-trough
> > statements in j1939tp_txnext (this isn't the easiest state machien on
> > Earth :-) ).
>
> Thanks.
>
> I have cherry picked both and will push them to v4.7. If everything
> works on the v4.7 branch, I'll squash most of it together an move to v4.8.
>
> Thoughts?
1) I'll try to move module parameters to /proc/sys/net/can-j1939/ items.
2) Tonight, I happen to meet an "open source license" expert.
I'll try to discuss with him the email address thing.
Kurt
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-06 9:37 UTC (permalink / raw)
To: linux-can, David Jander, Kurt Van Dijck
In-Reply-To: <20161006092403.GA26915@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 1210 bytes --]
On 10/06/2016 11:24 AM, Kurt Van Dijck wrote:
>>> * e2447ef: make TP data transmission work again
>>> This commit fixes a mistake you've done with the fall-trough
>>> statements in j1939tp_txnext (this isn't the easiest state machien on
>>> Earth :-) ).
>>
>> Thanks.
>>
>> I have cherry picked both and will push them to v4.7. If everything
>> works on the v4.7 branch, I'll squash most of it together an move to v4.8.
>>
>> Thoughts?
>
> 1) I'll try to move module parameters to /proc/sys/net/can-j1939/ items.
Tnx.
> 2) Tonight, I happen to meet an "open source license" expert.
> I'll try to discuss with him the email address thing.
IANAL: I think the tool pick up the Author of the commit. The (c)
copyright line is not relevant for tools, only humans...and lawyers.
Have a look at the .mailmap feature for author and/or email address
mapping: https://git-scm.com/docs/git-shortlog
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Measuring latency of CAN messages in SocketCAN
From: Bremer Henning @ 2016-10-06 8:37 UTC (permalink / raw)
To: linux-can
Hello,
I am using SocketCAN on a Beaglebone Black with Debian (Wheezy). I
have my Beaglebone plugged in a CAN-Network of 5 microcontrollers. Now
I would like to measure the time one message needs to be delivered
from my beaglebone to another microcontroller via CAN depending on the
priority (ID) of the sent message.
Is there a way to do this in SocketCAN?
Thank you!
Best regards
Henning
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-06 10:26 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, David Jander, Kurt Van Dijck
In-Reply-To: <9472847f-aa01-f4b0-f2cc-968a04c6ce06@pengutronix.de>
> On 10/06/2016 11:24 AM, Kurt Van Dijck wrote:
> >>> * e2447ef: make TP data transmission work again
> >>> This commit fixes a mistake you've done with the fall-trough
> >>> statements in j1939tp_txnext (this isn't the easiest state machien on
> >>> Earth :-) ).
> >>
> >> Thanks.
> >>
> >> I have cherry picked both and will push them to v4.7. If everything
> >> works on the v4.7 branch, I'll squash most of it together an move to v4.8.
> >>
> >> Thoughts?
> >
> > 1) I'll try to move module parameters to /proc/sys/net/can-j1939/ items.
I'm done.
I fixed the Makefile so it can compile can-j1939 as module again :-)
I changed the defaults to strict j1939 compliant (way-to-large max
packet size, padding).
And I migrated the module parameters to /proc/sys/net/can-j1939.
I loaded the module, and verified /proc, without any real wire tests.
So you need to cherry-pick another 3 commits.
Kind regards,
Kurt
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-06 10:28 UTC (permalink / raw)
To: linux-can, David Jander, Kurt Van Dijck
In-Reply-To: <20161006102643.GB26915@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 1328 bytes --]
On 10/06/2016 12:26 PM, Kurt Van Dijck wrote:
>> On 10/06/2016 11:24 AM, Kurt Van Dijck wrote:
>>>>> * e2447ef: make TP data transmission work again
>>>>> This commit fixes a mistake you've done with the fall-trough
>>>>> statements in j1939tp_txnext (this isn't the easiest state machien on
>>>>> Earth :-) ).
>>>>
>>>> Thanks.
>>>>
>>>> I have cherry picked both and will push them to v4.7. If everything
>>>> works on the v4.7 branch, I'll squash most of it together an move to v4.8.
>>>>
>>>> Thoughts?
>>>
>>> 1) I'll try to move module parameters to /proc/sys/net/can-j1939/ items.
>
> I'm done.
> I fixed the Makefile so it can compile can-j1939 as module again :-)
> I changed the defaults to strict j1939 compliant (way-to-large max
> packet size, padding).
> And I migrated the module parameters to /proc/sys/net/can-j1939.
> I loaded the module, and verified /proc, without any real wire tests.
>
> So you need to cherry-pick another 3 commits.
Tnx. Does it compile with disabled PROC support?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-06 11:13 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, David Jander, Kurt Van Dijck
In-Reply-To: <7a910650-0d91-e98c-8694-2d8b32ae68a7@pengutronix.de>
> On 10/06/2016 12:26 PM, Kurt Van Dijck wrote:
> >> On 10/06/2016 11:24 AM, Kurt Van Dijck wrote:
> >>>>> * e2447ef: make TP data transmission work again
> >>>>> This commit fixes a mistake you've done with the fall-trough
> >>>>> statements in j1939tp_txnext (this isn't the easiest state machien on
> >>>>> Earth :-) ).
> >>>>
> >>>> Thanks.
> >>>>
> >>>> I have cherry picked both and will push them to v4.7. If everything
> >>>> works on the v4.7 branch, I'll squash most of it together an move to v4.8.
> >>>>
> >>>> Thoughts?
> >>>
> >>> 1) I'll try to move module parameters to /proc/sys/net/can-j1939/ items.
> >
> > I'm done.
> > I fixed the Makefile so it can compile can-j1939 as module again :-)
> > I changed the defaults to strict j1939 compliant (way-to-large max
> > packet size, padding).
> > And I migrated the module parameters to /proc/sys/net/can-j1939.
> > I loaded the module, and verified /proc, without any real wire tests.
> >
> > So you need to cherry-pick another 3 commits.
>
> Tnx. Does it compile with disabled PROC support?
Yes, it just compiled (the module), not sure how it reacts when actually
used like that.
I just think I forgot to add descriptions in Documentation/sysctl.
>
> Marc
>
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
^ permalink raw reply
* Re: j1939
From: Kurt Van Dijck @ 2016-10-06 11:32 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can, David Jander
In-Reply-To: <20161006111347.GC26915@airbook.vandijck-laurijssen.be>
> > >>>
> > >>> 1) I'll try to move module parameters to /proc/sys/net/can-j1939/ items.
> > >
> > > I'm done.
> > > I fixed the Makefile so it can compile can-j1939 as module again :-)
> > > I changed the defaults to strict j1939 compliant (way-to-large max
> > > packet size, padding).
> > > And I migrated the module parameters to /proc/sys/net/can-j1939.
> > > I loaded the module, and verified /proc, without any real wire tests.
> > >
> > > So you need to cherry-pick another 3 commits.
> >
> > Tnx. Does it compile with disabled PROC support?
>
> Yes, it just compiled (the module), not sure how it reacts when actually
> used like that.
> I just think I forgot to add descriptions in Documentation/sysctl.
I added yet another commit, that add documentation of the new sysctl's.
Kurt
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-10-06 12:13 UTC (permalink / raw)
To: linux-can, David Jander
In-Reply-To: <20161006113244.GD26915@airbook.vandijck-laurijssen.be>
[-- Attachment #1.1: Type: text/plain, Size: 1211 bytes --]
On 10/06/2016 01:32 PM, Kurt Van Dijck wrote:
>>>>>>
>>>>>> 1) I'll try to move module parameters to /proc/sys/net/can-j1939/ items.
>>>>
>>>> I'm done.
>>>> I fixed the Makefile so it can compile can-j1939 as module again :-)
>>>> I changed the defaults to strict j1939 compliant (way-to-large max
>>>> packet size, padding).
>>>> And I migrated the module parameters to /proc/sys/net/can-j1939.
>>>> I loaded the module, and verified /proc, without any real wire tests.
>>>>
>>>> So you need to cherry-pick another 3 commits.
>>>
>>> Tnx. Does it compile with disabled PROC support?
>>
>> Yes, it just compiled (the module), not sure how it reacts when actually
>> used like that.
>> I just think I forgot to add descriptions in Documentation/sysctl.
>
> I added yet another commit, that add documentation of the new sysctl's.
Thanks - Cherry-picked and added two incremental patches on my branch.
Marc.
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox