* patch: namsiz
@ 2005-01-16 18:35 Jamal Hadi Salim
2005-01-16 18:49 ` Patrick McHardy
0 siblings, 1 reply; 11+ messages in thread
From: Jamal Hadi Salim @ 2005-01-16 18:35 UTC (permalink / raw)
To: David S. Miller; +Cc: Patrick McHardy, netdev
[-- Attachment #1: Type: text/plain, Size: 151 bytes --]
heres one ive wanted to do for a while - dissociate action name size
from ifnamsiz. for now set it to namize, but later may change it.
cheers,
jamal
[-- Attachment #2: bl_p --]
[-- Type: text/plain, Size: 1153 bytes --]
--- a/include/net/act_api.h 2005/01/16 13:23:00 1.1
+++ b/include/net/act_api.h 2005/01/16 13:24:13
@@ -40,6 +40,7 @@
#define ACT_P_CREATED 1
#define ACT_P_DELETED 1
+#define ANAMSIZ IFNAMSIZ
struct tcf_act_hdr
{
--- a/net/sched/act_api.c 2005/01/16 13:21:21 1.1
+++ b/net/sched/act_api.c 2005/01/16 13:24:33
@@ -227,7 +227,7 @@
if (a->ops == NULL || a->ops->dump == NULL)
return err;
- RTA_PUT(skb, TCA_KIND, IFNAMSIZ, a->ops->kind);
+ RTA_PUT(skb, TCA_KIND, ANAMSIZ, a->ops->kind);
if (tcf_action_copy_stats(skb, a))
goto rtattr_failure;
r = (struct rtattr*) skb->tail;
@@ -272,7 +272,7 @@
{
struct tc_action *a;
struct tc_action_ops *a_o;
- char act_name[IFNAMSIZ];
+ char act_name[ANAMSIZ];
struct rtattr *tb[TCA_ACT_MAX+1];
struct rtattr *kind;
@@ -284,10 +284,10 @@
kind = tb[TCA_ACT_KIND-1];
if (kind == NULL)
goto err_out;
- if (rtattr_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
+ if (rtattr_strlcpy(act_name, kind, ANAMSIZ) >= ANAMSIZ)
goto err_out;
} else {
- if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ)
+ if (strlcpy(act_name, name, ANAMSIZ) >= ANAMSIZ)
goto err_out;
}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: patch: namsiz 2005-01-16 18:35 patch: namsiz Jamal Hadi Salim @ 2005-01-16 18:49 ` Patrick McHardy 2005-01-16 18:56 ` Thomas Graf 0 siblings, 1 reply; 11+ messages in thread From: Patrick McHardy @ 2005-01-16 18:49 UTC (permalink / raw) To: hadi; +Cc: David S. Miller, netdev Jamal Hadi Salim wrote: >heres one ive wanted to do for a while - dissociate action name size >from ifnamsiz. for now set it to namize, but later may change it. > The same can be done for sch_api and cls_api. I'll add it to my tree when I'm done with the current fixes, unfortunately one of them has to touch basically all files in net/sched and I want to avoid clashes. Regards Patrick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: patch: namsiz 2005-01-16 18:49 ` Patrick McHardy @ 2005-01-16 18:56 ` Thomas Graf 2005-01-16 19:17 ` Jamal Hadi Salim 0 siblings, 1 reply; 11+ messages in thread From: Thomas Graf @ 2005-01-16 18:56 UTC (permalink / raw) To: Patrick McHardy; +Cc: hadi, David S. Miller, netdev * Patrick McHardy <41EAB74F.6060507@trash•net> 2005-01-16 19:49 > Jamal Hadi Salim wrote: > > >heres one ive wanted to do for a while - dissociate action name size > >from ifnamsiz. for now set it to namize, but later may change it. > > > The same can be done for sch_api and cls_api. I'll add it to my tree > when I'm done with the current fixes, unfortunately one of them has > to touch basically all files in net/sched and I want to avoid clashes. Go ahead, I have no changes in my tree except in a few header files and the necessary iproute2 changes will occupy me for a while. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: patch: namsiz 2005-01-16 18:56 ` Thomas Graf @ 2005-01-16 19:17 ` Jamal Hadi Salim 2005-01-16 19:32 ` path: module replay jamal 0 siblings, 1 reply; 11+ messages in thread From: Jamal Hadi Salim @ 2005-01-16 19:17 UTC (permalink / raw) To: Thomas Graf; +Cc: Patrick McHardy, David S. Miller, netdev On my part - theres one little change i made - just compiling it as we speak; I will post and stop there until you are done cheers, jamal On Sun, 2005-01-16 at 13:56, Thomas Graf wrote: > * Patrick McHardy <41EAB74F.6060507@trash•net> 2005-01-16 19:49 > > Jamal Hadi Salim wrote: > > > > >heres one ive wanted to do for a while - dissociate action name size > > >from ifnamsiz. for now set it to namize, but later may change it. > > > > > The same can be done for sch_api and cls_api. I'll add it to my tree > > when I'm done with the current fixes, unfortunately one of them has > > to touch basically all files in net/sched and I want to avoid clashes. > > Go ahead, I have no changes in my tree except in a few header files > and the necessary iproute2 changes will occupy me for a while. > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* path: module replay 2005-01-16 19:17 ` Jamal Hadi Salim @ 2005-01-16 19:32 ` jamal 2005-01-16 19:43 ` Patrick McHardy 2005-01-19 4:26 ` Patrick McHardy 0 siblings, 2 replies; 11+ messages in thread From: jamal @ 2005-01-16 19:32 UTC (permalink / raw) To: Thomas Graf; +Cc: Patrick McHardy, David S. Miller, netdev [-- Attachment #1: Type: text/plain, Size: 829 bytes --] Sorry, that work address just decides to appear when it feels like it. Doing this from my laptop which is also used for work (as well as extra curicular activities ;->). Now if evolution could setup the From address for me when posting to netdev it would help ;-> On Sun, 2005-01-16 at 14:17, Jamal Hadi Salim wrote: > On my part - theres one little change i made - just compiling it as we > speak; > I will post and stop there until you are done Ok, here is the last patch. I think module replay should be done from that spot and to be consistent as well from cls_api.c for legacy stuff. I also fixed a module ref count leak. the act_api piece is dependent on what i sent earlier for namsiz. the cls_api change i believe conflicts with what Thomas sent yesterday. Anyways, not sending anymore after this ;-> cheers, jamal [-- Attachment #2: p_replay --] [-- Type: text/plain, Size: 1680 bytes --] --- a/net/sched/act_api.c 2005/01/16 14:05:37 1.2 +++ b/net/sched/act_api.c 2005/01/16 14:22:47 @@ -308,7 +308,7 @@ */ if (a_o != NULL) { *err = -EAGAIN; - goto err_mod; + goto err_out; } #endif goto err_out; @@ -361,9 +361,13 @@ } for (i=0; i < TCA_ACT_MAX_PRIO && tb[i]; i++) { +replay: act = tcf_action_init_1(tb[i], est, name, ovr, bind, err); - if (act == NULL) - goto err; + if (act == NULL) { + if (*err == -EAGAIN) + goto replay; + goto err_out; + } act->order = i+1; if (head == NULL) @@ -374,7 +378,7 @@ } return head; -err: +err_out: if (head != NULL) tcf_action_destroy(head, bind); return NULL; @@ -752,10 +756,7 @@ */ if (n->nlmsg_flags&NLM_F_REPLACE) ovr = 1; -replay: ret = tcf_action_add(tca[TCA_ACT_TAB-1], n, pid, ovr); - if (ret == -EAGAIN) - goto replay; break; case RTM_DELACTION: ret = tca_action_gd(tca[TCA_ACT_TAB-1], n, pid, RTM_DELACTION); --- a/net/sched/cls_api.c 2005/01/16 14:09:39 1.1 +++ b/net/sched/cls_api.c 2005/01/16 14:20:41 @@ -486,14 +486,19 @@ memset(exts, 0, sizeof(*exts)); #ifdef CONFIG_NET_CLS_ACT + { int err; struct tc_action *act; if (map->police && tb[map->police-1]) { +replay: act = tcf_action_init_1(tb[map->police-1], rate_tlv, "police", TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err); - if (act == NULL) - return err; + if (act == NULL) { + if (err == -EAGAIN) + goto replay; + return err; + } act->type = TCA_OLD_COMPAT; exts->action = act; @@ -505,6 +510,7 @@ exts->action = act; } + } #elif defined CONFIG_NET_CLS_POLICE if (map->police && tb[map->police-1]) { struct tcf_police *p; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: path: module replay 2005-01-16 19:32 ` path: module replay jamal @ 2005-01-16 19:43 ` Patrick McHardy 2005-01-16 19:50 ` Patrick McHardy 2005-01-19 4:26 ` Patrick McHardy 1 sibling, 1 reply; 11+ messages in thread From: Patrick McHardy @ 2005-01-16 19:43 UTC (permalink / raw) To: hadi; +Cc: Thomas Graf, David S. Miller, netdev jamal wrote: >Ok, here is the last patch. I think module replay should be done from >that spot and to be consistent as well from cls_api.c for legacy stuff. >I also fixed a module ref count leak. >the act_api piece is dependent on what i sent earlier for namsiz. >the cls_api change i believe conflicts with what Thomas sent yesterday. > We have to replay all orders since we dropped the rtnl. An action could have done __dev_get_by_* or something similar. Can you send a single patch for the module refcount leak please ? Regards Patrick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: path: module replay 2005-01-16 19:43 ` Patrick McHardy @ 2005-01-16 19:50 ` Patrick McHardy 0 siblings, 0 replies; 11+ messages in thread From: Patrick McHardy @ 2005-01-16 19:50 UTC (permalink / raw) To: hadi; +Cc: Thomas Graf, David S. Miller, netdev Patrick McHardy wrote: > jamal wrote: > >> Ok, here is the last patch. I think module replay should be done from >> that spot and to be consistent as well from cls_api.c for legacy stuff. >> I also fixed a module ref count leak. the act_api piece is dependent >> on what i sent earlier for namsiz. >> the cls_api change i believe conflicts with what Thomas sent yesterday. >> > We have to replay all orders since we dropped the rtnl. An action > could have done __dev_get_by_* or something similar. Can you send > a single patch for the module refcount leak please ? I was wrong, the already initialized actions need to hold their own references anyway. I'm going have another look and add your patch later if I don't see other problems. Regards Patrick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: path: module replay 2005-01-16 19:32 ` path: module replay jamal 2005-01-16 19:43 ` Patrick McHardy @ 2005-01-19 4:26 ` Patrick McHardy 2005-01-19 13:35 ` jamal 2005-01-24 13:28 ` jamal 1 sibling, 2 replies; 11+ messages in thread From: Patrick McHardy @ 2005-01-19 4:26 UTC (permalink / raw) To: hadi; +Cc: Thomas Graf, David S. Miller, netdev jamal wrote: >Ok, here is the last patch. I think module replay should be done from >that spot and to be consistent as well from cls_api.c for legacy stuff. >I also fixed a module ref count leak. >the act_api piece is dependent on what i sent earlier for namsiz. >the cls_api change i believe conflicts with what Thomas sent yesterday. > +replay: act = tcf_action_init_1(tb[i], est, name, ovr, bind, err); - if (act == NULL) - goto err; + if (act == NULL) { + if (*err == -EAGAIN) + goto replay; + goto err_out; + } This part is wrong. We can replay single action requests in the act_api init path, but not in tcf_exts_init. We are coming from a classifier initialization path and have dropped the RTNL, so we also have to replay the classifier request. Please send a fixed patch without the bogus module refcnt change. Regards Patrick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: path: module replay 2005-01-19 4:26 ` Patrick McHardy @ 2005-01-19 13:35 ` jamal 2005-01-19 14:02 ` Patrick McHardy 2005-01-24 13:28 ` jamal 1 sibling, 1 reply; 11+ messages in thread From: jamal @ 2005-01-19 13:35 UTC (permalink / raw) To: Patrick McHardy; +Cc: Thomas Graf, David S. Miller, netdev On Tue, 2005-01-18 at 23:26, Patrick McHardy wrote: > jamal wrote: > > >Ok, here is the last patch. I think module replay should be done from > >that spot and to be consistent as well from cls_api.c for legacy stuff. > >I also fixed a module ref count leak. > >the act_api piece is dependent on what i sent earlier for namsiz. > >the cls_api change i believe conflicts with what Thomas sent yesterday. > > > +replay: > act = tcf_action_init_1(tb[i], est, name, ovr, bind, err); > - if (act == NULL) > - goto err; > + if (act == NULL) { > + if (*err == -EAGAIN) > + goto replay; > + goto err_out; > + } > > This part is wrong. And we exchanged emails on this topic already in private and you said you will fix and send out the patch;-> So please do just that or you can wait until i have more time and my hardware at the same spot. cheers, jamal ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: path: module replay 2005-01-19 13:35 ` jamal @ 2005-01-19 14:02 ` Patrick McHardy 0 siblings, 0 replies; 11+ messages in thread From: Patrick McHardy @ 2005-01-19 14:02 UTC (permalink / raw) To: hadi; +Cc: Thomas Graf, David S. Miller, netdev jamal wrote: > >And we exchanged emails on this topic already in private and you said >you will fix and send out the patch;-> So please do just that or you can >wait until i have more time and my hardware at the same spot. > I said I'm going to remove the bogus module loading changes and apply the patch if there aren't more problems. But there are, and my time is limited too, so I'm going to wait for a new patch. Regards Patrick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: path: module replay 2005-01-19 4:26 ` Patrick McHardy 2005-01-19 13:35 ` jamal @ 2005-01-24 13:28 ` jamal 1 sibling, 0 replies; 11+ messages in thread From: jamal @ 2005-01-24 13:28 UTC (permalink / raw) To: Patrick McHardy; +Cc: Thomas Graf, David S. Miller, netdev On Tue, 2005-01-18 at 23:26, Patrick McHardy wrote: > jamal wrote: > > >Ok, here is the last patch. I think module replay should be done from > >that spot and to be consistent as well from cls_api.c for legacy stuff. > >I also fixed a module ref count leak. > >the act_api piece is dependent on what i sent earlier for namsiz. > >the cls_api change i believe conflicts with what Thomas sent yesterday. > > > +replay: > act = tcf_action_init_1(tb[i], est, name, ovr, bind, err); > - if (act == NULL) > - goto err; > + if (act == NULL) { > + if (*err == -EAGAIN) > + goto replay; > + goto err_out; > + } > > This part is wrong. We can replay single action requests in the act_api init > path, but not in tcf_exts_init. We are coming from a classifier > initialization > path and have dropped the RTNL, so we also have to replay the classifier > request. > > Please send a fixed patch without the bogus module refcnt change. > I just upgraded to rc2 + latest bk - compiling - and looking at that path again; i think what you have already is much cleaner. We may end up getting a shorter code path for the non-classifier path in what i suggested, but its not worth the cosmetic change. The one thing that would be valuable to change is that goto i.e --- -err: +err_out: --- since err is also a variable name. But i dont think this in itself warrants a patch; maybe another patch that comes along later for something else also takes care of this. cheers, jamal ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2005-01-24 13:28 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-01-16 18:35 patch: namsiz Jamal Hadi Salim 2005-01-16 18:49 ` Patrick McHardy 2005-01-16 18:56 ` Thomas Graf 2005-01-16 19:17 ` Jamal Hadi Salim 2005-01-16 19:32 ` path: module replay jamal 2005-01-16 19:43 ` Patrick McHardy 2005-01-16 19:50 ` Patrick McHardy 2005-01-19 4:26 ` Patrick McHardy 2005-01-19 13:35 ` jamal 2005-01-19 14:02 ` Patrick McHardy 2005-01-24 13:28 ` jamal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox