* can-next pull request for CAN ID Ematch
@ 2012-07-04 4:11 Oliver Hartkopp
2012-07-04 6:03 ` Oliver Hartkopp
0 siblings, 1 reply; 4+ messages in thread
From: Oliver Hartkopp @ 2012-07-04 4:11 UTC (permalink / raw)
To: Marc Kleine-Budde, Rostislav Lisovy
Cc: linux-can, Pavel Píša, Michal Sojka
Hello Marc, hello Rostislav,
as Rostislav seems to be absent, i fixed the minor issues myself and created
a branch for that in my ollis-can-next git repo.
FYI the changes can be seen in the patch at the end of this mail.
The following changes since commit 0e1aa9cfe6db4689f98b20afe808b4f9cdb0b6c0:
em_canid: Ematch rule to match CAN frames according to their identifiers (2012-07-04 05:32:03 +0200)
are available in the git repository at:
git://gitorious.org/~hartkopp/linux-can/ollis-can-next.git em_can
for you to fetch changes up to 0e1aa9cfe6db4689f98b20afe808b4f9cdb0b6c0:
em_canid: Ematch rule to match CAN frames according to their identifiers (2012-07-04 05:32:03 +0200)
diff --git a/net/sched/em_canid.c b/net/sched/em_canid.c
index d607e78..dc9f515 100644
--- a/net/sched/em_canid.c
+++ b/net/sched/em_canid.c
@@ -123,13 +123,10 @@ static int em_canid_match(struct sk_buff *skb, struct tcf_ematch *m,
static int em_canid_change(struct tcf_proto *tp, void *data, int len,
struct tcf_ematch *m)
{
- struct can_filter *conf = data; /* Array with rules,
- * fixed size EM_CAN_RULES_SIZE
- */
+ struct can_filter *conf = data; /* Array with rules */
struct canid_match *cm;
struct canid_match *cm_old = (struct canid_match *)m->data;
int i;
- int rulescnt;
if (!len)
return -EINVAL;
@@ -140,43 +137,37 @@ static int em_canid_change(struct tcf_proto *tp, void *data, int len,
if (len > sizeof(struct can_filter) * EM_CAN_RULES_MAX)
return -EINVAL;
- rulescnt = len / sizeof(struct can_filter);
-
- cm = kzalloc(sizeof(struct canid_match) + sizeof(struct can_filter) *
- rulescnt, GFP_KERNEL);
+ cm = kzalloc(sizeof(struct canid_match) + len, GFP_KERNEL);
if (!cm)
return -ENOMEM;
- cm->sff_rules_count = 0;
- cm->eff_rules_count = 0;
- cm->rules_count = rulescnt;
+ cm->rules_count = len / sizeof(struct can_filter);
/*
* We need two for() loops for copying rules into
* two contiguous areas in rules_raw
*/
- /* Process EFF frame rules*/
+ /* Process EFF and mixed EFF/SFF frame rules*/
for (i = 0; i < cm->rules_count; i++) {
- if (((conf[i].can_id & CAN_EFF_FLAG) &&
- (conf[i].can_mask & CAN_EFF_FLAG)) ||
- !(conf[i].can_mask & CAN_EFF_FLAG)) {
+ if (!(conf[i].can_id & CAN_EFF_FLAG) &&
+ (conf[i].can_mask & CAN_EFF_FLAG)) {
+ /* SFF only filter */
+ continue;
+ } else {
memcpy(cm->rules_raw + cm->eff_rules_count,
&conf[i],
sizeof(struct can_filter));
cm->eff_rules_count++;
- } else {
- continue;
}
}
/* Process SFF frame rules */
for (i = 0; i < cm->rules_count; i++) {
- if ((conf[i].can_id & CAN_EFF_FLAG) &&
+ if (!(conf[i].can_id & CAN_EFF_FLAG) &&
(conf[i].can_mask & CAN_EFF_FLAG)) {
- continue;
- } else {
+ /* SFF only filter */
memcpy(cm->rules_raw
+ cm->eff_rules_count
+ cm->sff_rules_count,
@@ -186,10 +177,12 @@ static int em_canid_change(struct tcf_proto *tp, void *data, int len,
em_canid_sff_match_add(cm,
conf[i].can_id, conf[i].can_mask);
+ } else {
+ continue;
}
}
- m->datalen = sizeof(*cm);
+ m->datalen = sizeof(struct canid_match) + len;
m->data = (unsigned long)cm;
if (cm_old != NULL) {
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: can-next pull request for CAN ID Ematch
2012-07-04 4:11 can-next pull request for CAN ID Ematch Oliver Hartkopp
@ 2012-07-04 6:03 ` Oliver Hartkopp
2012-07-04 11:09 ` Marc Kleine-Budde
0 siblings, 1 reply; 4+ messages in thread
From: Oliver Hartkopp @ 2012-07-04 6:03 UTC (permalink / raw)
To: Marc Kleine-Budde, Rostislav Lisovy
Cc: linux-can, Pavel Píša, Michal Sojka
Hi all,
the pull location is still valid :-)
Btw. i needed to change a minor thing as the em_canid_match() function did
not handle the case that the same CAN-ID can be SFF or EFF ...
The option to support the CAN_EFF_FLAG cleared in the can_mask is broken,
unusual and can not be handled in a simple way.
The tc tool can handle eff and sff rules - and so i did it in the
em_canid_change() function now. No support for CAN_EFF_FLAG cleared in the
can_mask - just check the CAN_EFF_FLAG in the filter.can_id to decide the
rules type.
See at line 146ff here:
https://gitorious.org/~hartkopp/linux-can/ollis-can-next/commit/e8edbabb5955e1444f69136eae2f52889a42b153
Additional i kicked out the pointless "else { continue; }" statements 8-)
Regards,
Oliver
On 04.07.2012 06:11, Oliver Hartkopp wrote:
> Hello Marc, hello Rostislav,
>
> as Rostislav seems to be absent, i fixed the minor issues myself and created
> a branch for that in my ollis-can-next git repo.
>
> FYI the changes can be seen in the patch at the end of this mail.
>
>
> The following changes since commit 0e1aa9cfe6db4689f98b20afe808b4f9cdb0b6c0:
>
> em_canid: Ematch rule to match CAN frames according to their identifiers (2012-07-04 05:32:03 +0200)
>
> are available in the git repository at:
>
> git://gitorious.org/~hartkopp/linux-can/ollis-can-next.git em_can
>
> for you to fetch changes up to 0e1aa9cfe6db4689f98b20afe808b4f9cdb0b6c0:
>
> em_canid: Ematch rule to match CAN frames according to their identifiers (2012-07-04 05:32:03 +0200)
>
>
> diff --git a/net/sched/em_canid.c b/net/sched/em_canid.c
> index d607e78..dc9f515 100644
> --- a/net/sched/em_canid.c
> +++ b/net/sched/em_canid.c
> @@ -123,13 +123,10 @@ static int em_canid_match(struct sk_buff *skb, struct tcf_ematch *m,
> static int em_canid_change(struct tcf_proto *tp, void *data, int len,
> struct tcf_ematch *m)
> {
> - struct can_filter *conf = data; /* Array with rules,
> - * fixed size EM_CAN_RULES_SIZE
> - */
> + struct can_filter *conf = data; /* Array with rules */
> struct canid_match *cm;
> struct canid_match *cm_old = (struct canid_match *)m->data;
> int i;
> - int rulescnt;
>
> if (!len)
> return -EINVAL;
> @@ -140,43 +137,37 @@ static int em_canid_change(struct tcf_proto *tp, void *data, int len,
> if (len > sizeof(struct can_filter) * EM_CAN_RULES_MAX)
> return -EINVAL;
>
> - rulescnt = len / sizeof(struct can_filter);
> -
> - cm = kzalloc(sizeof(struct canid_match) + sizeof(struct can_filter) *
> - rulescnt, GFP_KERNEL);
> + cm = kzalloc(sizeof(struct canid_match) + len, GFP_KERNEL);
> if (!cm)
> return -ENOMEM;
>
> - cm->sff_rules_count = 0;
> - cm->eff_rules_count = 0;
> - cm->rules_count = rulescnt;
> + cm->rules_count = len / sizeof(struct can_filter);
>
> /*
> * We need two for() loops for copying rules into
> * two contiguous areas in rules_raw
> */
>
> - /* Process EFF frame rules*/
> + /* Process EFF and mixed EFF/SFF frame rules*/
> for (i = 0; i < cm->rules_count; i++) {
> - if (((conf[i].can_id & CAN_EFF_FLAG) &&
> - (conf[i].can_mask & CAN_EFF_FLAG)) ||
> - !(conf[i].can_mask & CAN_EFF_FLAG)) {
> + if (!(conf[i].can_id & CAN_EFF_FLAG) &&
> + (conf[i].can_mask & CAN_EFF_FLAG)) {
> + /* SFF only filter */
> + continue;
> + } else {
> memcpy(cm->rules_raw + cm->eff_rules_count,
> &conf[i],
> sizeof(struct can_filter));
>
> cm->eff_rules_count++;
> - } else {
> - continue;
> }
> }
>
> /* Process SFF frame rules */
> for (i = 0; i < cm->rules_count; i++) {
> - if ((conf[i].can_id & CAN_EFF_FLAG) &&
> + if (!(conf[i].can_id & CAN_EFF_FLAG) &&
> (conf[i].can_mask & CAN_EFF_FLAG)) {
> - continue;
> - } else {
> + /* SFF only filter */
> memcpy(cm->rules_raw
> + cm->eff_rules_count
> + cm->sff_rules_count,
> @@ -186,10 +177,12 @@ static int em_canid_change(struct tcf_proto *tp, void *data, int len,
>
> em_canid_sff_match_add(cm,
> conf[i].can_id, conf[i].can_mask);
> + } else {
> + continue;
> }
> }
>
> - m->datalen = sizeof(*cm);
> + m->datalen = sizeof(struct canid_match) + len;
> m->data = (unsigned long)cm;
>
> if (cm_old != NULL) {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: can-next pull request for CAN ID Ematch
2012-07-04 6:03 ` Oliver Hartkopp
@ 2012-07-04 11:09 ` Marc Kleine-Budde
2012-07-04 11:19 ` Oliver Hartkopp
0 siblings, 1 reply; 4+ messages in thread
From: Marc Kleine-Budde @ 2012-07-04 11:09 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Rostislav Lisovy, linux-can, Pavel Píša, Michal Sojka
[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]
On 07/04/2012 08:03 AM, Oliver Hartkopp wrote:
> Hi all,
>
> the pull location is still valid :-)
>
> Btw. i needed to change a minor thing as the em_canid_match() function did
> not handle the case that the same CAN-ID can be SFF or EFF ...
>
> The option to support the CAN_EFF_FLAG cleared in the can_mask is broken,
> unusual and can not be handled in a simple way.
>
> The tc tool can handle eff and sff rules - and so i did it in the
> em_canid_change() function now. No support for CAN_EFF_FLAG cleared in the
> can_mask - just check the CAN_EFF_FLAG in the filter.can_id to decide the
> rules type.
I hope I took the correct patch. Please check the for-davem branch in my
gitorious repo[1].
I've changed some things in the commit:
- credited Rostislav Lisovy as the original author.
(git commit --amend --author="Rostislav Lisovy <lisovy@gmail.com>")
- added a "net:" in the Subject line
- Fixed coding style issue in pkt_cls.h
all defines indent now with a tab
The patch currently has these S-o-bs:
Signed-off-by: Rostislav Lisovy <lisovy@gmail.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Oliver, if you want to credit your changes I can add a oneliner between
Rostislav's and your S-o-b, which may look like this:
[Oliver: fixed random stuff]
Marc
cheers, Marc
[1] https://gitorious.org/linux-can/linux-can-next/commits/for-davem
--
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: 262 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: can-next pull request for CAN ID Ematch
2012-07-04 11:09 ` Marc Kleine-Budde
@ 2012-07-04 11:19 ` Oliver Hartkopp
0 siblings, 0 replies; 4+ messages in thread
From: Oliver Hartkopp @ 2012-07-04 11:19 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Oliver Hartkopp, Rostislav Lisovy, linux-can,
Pavel Píša, Michal Sojka
On 04.07.2012 13:09, Marc Kleine-Budde wrote:
> On 07/04/2012 08:03 AM, Oliver Hartkopp wrote:
>> Hi all,
>>
>> the pull location is still valid :-)
>>
>> Btw. i needed to change a minor thing as the em_canid_match() function did
>> not handle the case that the same CAN-ID can be SFF or EFF ...
>>
>> The option to support the CAN_EFF_FLAG cleared in the can_mask is broken,
>> unusual and can not be handled in a simple way.
>>
>> The tc tool can handle eff and sff rules - and so i did it in the
>> em_canid_change() function now. No support for CAN_EFF_FLAG cleared in the
>> can_mask - just check the CAN_EFF_FLAG in the filter.can_id to decide the
>> rules type.
>
> I hope I took the correct patch. Please check the for-davem branch in my
> gitorious repo[1].
Yes - it is the correct patch.
>
> I've changed some things in the commit:
> - credited Rostislav Lisovy as the original author.
> (git commit --amend --author="Rostislav Lisovy<lisovy@gmail.com>")
> - added a "net:" in the Subject line
> - Fixed coding style issue in pkt_cls.h
> all defines indent now with a tab
Good!
I also had some whitespace issues in Kconfig i fixed this morning ...
>
> The patch currently has these S-o-bs:
>
> Signed-off-by: Rostislav Lisovy<lisovy@gmail.com>
> Signed-off-by: Oliver Hartkopp<socketcan@hartkopp.net>
> Signed-off-by: Marc Kleine-Budde<mkl@pengutronix.de>
>
> Oliver, if you want to credit your changes I can add a oneliner between
> Rostislav's and your S-o-b, which may look like this:
>
> [Oliver: fixed random stuff]
No need for that.
I'm pretty sure now that there are no obvious issues open now.
At least it's getting shorter and shorter it get's easier to review ;-)
Tnx!
Oliver
>
> [1] https://gitorious.org/linux-can/linux-can-next/commits/for-davem
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-07-04 11:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-04 4:11 can-next pull request for CAN ID Ematch Oliver Hartkopp
2012-07-04 6:03 ` Oliver Hartkopp
2012-07-04 11:09 ` Marc Kleine-Budde
2012-07-04 11:19 ` Oliver Hartkopp
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.