* [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
@ 2004-08-23 20:51 Thomas Graf
2004-08-24 2:26 ` Herbert Xu
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2004-08-23 20:51 UTC (permalink / raw)
To: davem, kuznet, hadi; +Cc: netdev
Hello
Introduces a new rtnetlink link message attribute to provide txqlen value
via RTM_GETLINK and adds support for setting MTU and txqlen via RTM_SETLINK.
First step to supersede ioctl network link configuration with
rtnetlink equivalent.
Thomas
--- 1.19/net/core/rtnetlink.c Thu Apr 29 01:04:48 2004
+++ 1.20/net/core/rtnetlink.c Sun Aug 22 18:11:03 2004
@@ -175,6 +175,7 @@
r->ifi_flags |= IFF_RUNNING;
RTA_PUT(skb, IFLA_IFNAME, strlen(dev->name)+1, dev->name);
+ RTA_PUT(skb, IFLA_TXQLEN, sizeof(unsigned long), &dev->tx_queue_len);
if (dev->addr_len) {
RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr);
RTA_PUT(skb, IFLA_BROADCAST, dev->addr_len, dev->broadcast);
@@ -268,6 +269,23 @@
goto out;
memcpy(dev->broadcast, RTA_DATA(ida[IFLA_BROADCAST - 1]),
dev->addr_len);
+ }
+
+ if (ida[IFLA_MTU - 1]) {
+ if (ida[IFLA_MTU - 1]->rta_len != RTA_LENGTH(sizeof(unsigned)))
+ goto out;
+ err = dev_set_mtu(dev, *((unsigned *) RTA_DATA(ida[IFLA_MTU - 1])));
+
+ if (err)
+ goto out;
+
+ }
+
+ if (ida[IFLA_TXQLEN - 1]) {
+ if (ida[IFLA_TXQLEN - 1]->rta_len != RTA_LENGTH(sizeof(unsigned long)))
+ goto out;
+
+ dev->tx_queue_len = *((unsigned long *) RTA_DATA(ida[IFLA_TXQLEN - 1]));
}
err = 0;
--- 1.33/include/linux/rtnetlink.h Mon Apr 5 23:41:40 2004
+++ 1.34/include/linux/rtnetlink.h Sun Aug 22 18:11:03 2004
@@ -540,6 +540,7 @@
IFLA_QDISC,
IFLA_STATS,
IFLA_COST,
+ IFLA_TXQLEN,
#define IFLA_COST IFLA_COST
IFLA_PRIORITY,
#define IFLA_PRIORITY IFLA_PRIORITY
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-23 20:51 [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK Thomas Graf
@ 2004-08-24 2:26 ` Herbert Xu
2004-08-24 9:49 ` Thomas Graf
0 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2004-08-24 2:26 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, kuznet, hadi, netdev
Thomas Graf <tgraf@suug.ch> wrote:
>
> + RTA_PUT(skb, IFLA_TXQLEN, sizeof(unsigned long), &dev->tx_queue_len);
Please use a fixed width type like u32. Keep in mind that the ioctl
version is exporting an int.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 2:26 ` Herbert Xu
@ 2004-08-24 9:49 ` Thomas Graf
2004-08-24 10:16 ` Herbert Xu
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2004-08-24 9:49 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, kuznet, hadi, netdev
* Herbert Xu <E1BzR1C-0000c4-00@gondolin.me.apana.org.au> 2004-08-24 12:26
> Thomas Graf <tgraf@suug.ch> wrote:
> >
> > + RTA_PUT(skb, IFLA_TXQLEN, sizeof(unsigned long), &dev->tx_queue_len);
>
> Please use a fixed width type like u32.
I used the same type as it is stored in struct net_device to avoid data
loss on certain architectures. Due to the fact that the size of the
attribute is provided to the userspace receiver, no guessing is involved.
Using u32 would mean to do an unnecessary cast.
> Keep in mind that the ioctl version is exporting an int.
Yes, but the size of ivalue is also dependent on the architecture and
the cast to int is done for compatibility reasons.
Thomas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 9:49 ` Thomas Graf
@ 2004-08-24 10:16 ` Herbert Xu
2004-08-24 12:04 ` Thomas Graf
2004-08-24 16:25 ` [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK Thomas Graf
0 siblings, 2 replies; 13+ messages in thread
From: Herbert Xu @ 2004-08-24 10:16 UTC (permalink / raw)
To: Thomas Graf; +Cc: herbert, davem, kuznet, hadi, netdev
Thomas Graf <tgraf@suug.ch> wrote:
> * Herbert Xu <E1BzR1C-0000c4-00@gondolin.me.apana.org.au> 2004-08-24 12:26
>> Thomas Graf <tgraf@suug.ch> wrote:
>> >
>> > + RTA_PUT(skb, IFLA_TXQLEN, sizeof(unsigned long), &dev->tx_queue_len);
>>
>> Please use a fixed width type like u32.
>
> I used the same type as it is stored in struct net_device to avoid data
> loss on certain architectures. Due to the fact that the size of the
> attribute is provided to the userspace receiver, no guessing is involved.
> Using u32 would mean to do an unnecessary cast.
Please think about the meaning of the value. Is anyone going to have a queue
bigger than 2^32?
And if the answer is yes, then please use u64.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 10:16 ` Herbert Xu
@ 2004-08-24 12:04 ` Thomas Graf
2004-08-24 17:32 ` David S. Miller
2004-08-24 16:25 ` [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK Thomas Graf
1 sibling, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2004-08-24 12:04 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, kuznet, hadi, netdev
* Herbert Xu <E1BzYMJ-000208-00@gondolin.me.apana.org.au> 2004-08-24 20:16
> Please think about the meaning of the value. Is anyone going to have a queue
> bigger than 2^32?
>
> And if the answer is yes, then please use u64.
Can you please explain the actual reason for using a fixed width
type when all existing numeric attributes use arch depedent types?
I agree with you, if it happens to be the reason, that it is easier for a
userspace application to use fixed width types but we should change
it for all attributes if the decision is made for this one.
I have more patches in my queue which would throw up the same question
again. Therefore I'd like to see it discussed now and a decision to be
made whether to use fixed width types or not.
Thomas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 10:16 ` Herbert Xu
2004-08-24 12:04 ` Thomas Graf
@ 2004-08-24 16:25 ` Thomas Graf
2004-08-24 17:32 ` David S. Miller
1 sibling, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2004-08-24 16:25 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, kuznet, hadi, netdev
* Herbert Xu <E1BzYMJ-000208-00@gondolin.me.apana.org.au> 2004-08-24 20:16
> Please think about the meaning of the value. Is anyone going to have a queue
> bigger than 2^32?
>
> And if the answer is yes, then please use u64.
Here's a new patch with all discussed changes integrated. I don't have enough
cycles to split it all up again.
Introduces IFLA_TXQLEN and IFLA_MAP to support accessing ifmap
and txqlen via RTM_(SET|GET)LINK. Converts IFLA_MTU, IFLA_LINK
and IFLA_MASTER from arch dependent types to fixed width types.
Adds a IFLA_MTU handler for RTM_SETLINK to support changing
the MTU via RTM_SETLINK. Adds a handler for ifi_flags to
support changing net_device flags via RTM_SETLINK and changes
RTM_GETLINK to be symmetric with RTM_SETLINK like in ioctl
handlers.
Thomas
--- 1.19/net/core/rtnetlink.c Thu Apr 29 01:04:48 2004
+++ 1.27/net/core/rtnetlink.c Tue Aug 24 18:16:40 2004
@@ -166,31 +166,53 @@
r->ifi_family = AF_UNSPEC;
r->ifi_type = dev->type;
r->ifi_index = dev->ifindex;
- r->ifi_flags = dev->flags;
+ r->ifi_flags = dev_get_flags(dev);
r->ifi_change = change;
- if (!netif_running(dev) || !netif_carrier_ok(dev))
- r->ifi_flags &= ~IFF_RUNNING;
- else
- r->ifi_flags |= IFF_RUNNING;
-
RTA_PUT(skb, IFLA_IFNAME, strlen(dev->name)+1, dev->name);
+
+ if (1) {
+ u32 txqlen = dev->tx_queue_len;
+ RTA_PUT(skb, IFLA_TXQLEN, sizeof(txqlen), &txqlen);
+ }
+
+ if (1) {
+ struct ifmap map = {
+ .mem_start = dev->mem_start,
+ .mem_end = dev->mem_end,
+ .base_addr = dev->base_addr,
+ .irq = dev->irq,
+ .dma = dev->dma,
+ .port = dev->if_port,
+ };
+ RTA_PUT(skb, IFLA_MAP, sizeof(map), &map);
+ }
+
if (dev->addr_len) {
RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr);
RTA_PUT(skb, IFLA_BROADCAST, dev->addr_len, dev->broadcast);
}
+
if (1) {
- unsigned mtu = dev->mtu;
+ u32 mtu = dev->mtu;
RTA_PUT(skb, IFLA_MTU, sizeof(mtu), &mtu);
}
- if (dev->ifindex != dev->iflink)
- RTA_PUT(skb, IFLA_LINK, sizeof(int), &dev->iflink);
+
+ if (dev->ifindex != dev->iflink) {
+ u32 iflink = dev->iflink;
+ RTA_PUT(skb, IFLA_LINK, sizeof(iflink), &iflink);
+ }
+
if (dev->qdisc_sleeping)
RTA_PUT(skb, IFLA_QDISC,
strlen(dev->qdisc_sleeping->ops->id) + 1,
dev->qdisc_sleeping->ops->id);
- if (dev->master)
- RTA_PUT(skb, IFLA_MASTER, sizeof(int), &dev->master->ifindex);
+
+ if (dev->master) {
+ u32 master = dev->master->ifindex;
+ RTA_PUT(skb, IFLA_MASTER, sizeof(master), &master);
+ }
+
if (dev->get_stats) {
unsigned long *stats = (unsigned long*)dev->get_stats(dev);
if (stats) {
@@ -246,6 +268,30 @@
err = -EINVAL;
+ if (ifm->ifi_flags)
+ dev_change_flags(dev, ifm->ifi_flags);
+
+ if (ida[IFLA_MAP - 1]) {
+ if (!dev->set_config) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
+
+ if (!netif_device_present(dev)) {
+ err = -ENODEV;
+ goto out;
+ }
+
+ if (ida[IFLA_MAP - 1]->rta_len != RTA_LENGTH(sizeof(struct ifmap)))
+ goto out;
+
+ err = dev->set_config(dev, (struct ifmap *)
+ RTA_DATA(ida[IFLA_MAP - 1]));
+
+ if (err)
+ goto out;
+ }
+
if (ida[IFLA_ADDRESS - 1]) {
if (!dev->set_mac_address) {
err = -EOPNOTSUPP;
@@ -268,6 +314,23 @@
goto out;
memcpy(dev->broadcast, RTA_DATA(ida[IFLA_BROADCAST - 1]),
dev->addr_len);
+ }
+
+ if (ida[IFLA_MTU - 1]) {
+ if (ida[IFLA_MTU - 1]->rta_len != RTA_LENGTH(sizeof(u32)))
+ goto out;
+ err = dev_set_mtu(dev, *((u32 *) RTA_DATA(ida[IFLA_MTU - 1])));
+
+ if (err)
+ goto out;
+
+ }
+
+ if (ida[IFLA_TXQLEN - 1]) {
+ if (ida[IFLA_TXQLEN - 1]->rta_len != RTA_LENGTH(sizeof(u32)))
+ goto out;
+
+ dev->tx_queue_len = *((u32 *) RTA_DATA(ida[IFLA_TXQLEN - 1]));
}
err = 0;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 12:04 ` Thomas Graf
@ 2004-08-24 17:32 ` David S. Miller
2004-08-24 18:15 ` Thomas Graf
2004-08-24 20:47 ` [RFC/PATCH] Convert architecture dependent rtnetlink attribute to fixed sized ones Thomas Graf
0 siblings, 2 replies; 13+ messages in thread
From: David S. Miller @ 2004-08-24 17:32 UTC (permalink / raw)
To: Thomas Graf; +Cc: herbert, kuznet, hadi, netdev
On Tue, 24 Aug 2004 14:04:42 +0200
Thomas Graf <tgraf@suug.ch> wrote:
> * Herbert Xu <E1BzYMJ-000208-00@gondolin.me.apana.org.au> 2004-08-24 20:16
> > Please think about the meaning of the value. Is anyone going to have a queue
> > bigger than 2^32?
> >
> > And if the answer is yes, then please use u64.
>
> Can you please explain the actual reason for using a fixed width
> type when all existing numeric attributes use arch depedent types?
Because otherwise things will break and explode for 32-bit
binaries running on 64-bit kernels, which is the situation
for the majority of userland on some platforms.
We have a compatability layer to translate 32-bit system
call structures to/from 64-bit data types. But such
a scheme does not work for things like netlink sockets
where it is the data stream via read/write calls that
contains the structures.
Also, we eventually want to be able to transmit netlink
messages over real networks to remote clients.
Therefore, like any other real network protocol, we should
use fixed sized types in the data.
Stop bickering and use a u32, or I'll make that change for
you :-)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 16:25 ` [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK Thomas Graf
@ 2004-08-24 17:32 ` David S. Miller
2004-08-24 18:08 ` Thomas Graf
0 siblings, 1 reply; 13+ messages in thread
From: David S. Miller @ 2004-08-24 17:32 UTC (permalink / raw)
To: Thomas Graf; +Cc: herbert, kuznet, hadi, netdev
On Tue, 24 Aug 2004 18:25:01 +0200
Thomas Graf <tgraf@suug.ch> wrote:
> * Herbert Xu <E1BzYMJ-000208-00@gondolin.me.apana.org.au> 2004-08-24 20:16
> > Please think about the meaning of the value. Is anyone going to have a queue
> > bigger than 2^32?
> >
> > And if the answer is yes, then please use u64.
>
> Here's a new patch with all discussed changes integrated. I don't have enough
> cycles to split it all up again.
This patch only contained net/core/rtnetlink.c changes, where is
the rest?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 17:32 ` David S. Miller
@ 2004-08-24 18:08 ` Thomas Graf
2004-08-25 0:34 ` David S. Miller
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2004-08-24 18:08 UTC (permalink / raw)
To: David S. Miller; +Cc: herbert, kuznet, hadi, netdev
* David S. Miller <20040824103252.1b593e68.davem@redhat.com> 2004-08-24 10:32
> On Tue, 24 Aug 2004 18:25:01 +0200
> Thomas Graf <tgraf@suug.ch> wrote:
>
> > * Herbert Xu <E1BzYMJ-000208-00@gondolin.me.apana.org.au> 2004-08-24 20:16
> > > Please think about the meaning of the value. Is anyone going to have a queue
> > > bigger than 2^32?
> > >
> > > And if the answer is yes, then please use u64.
> >
> > Here's a new patch with all discussed changes integrated. I don't have enough
> > cycles to split it all up again.
>
> This patch only contained net/core/rtnetlink.c changes, where is
> the rest?
Sorry, the complete patch again:
--- 1.19/net/core/rtnetlink.c Thu Apr 29 01:04:48 2004
+++ 1.27/net/core/rtnetlink.c Tue Aug 24 18:16:40 2004
@@ -166,31 +166,53 @@
r->ifi_family = AF_UNSPEC;
r->ifi_type = dev->type;
r->ifi_index = dev->ifindex;
- r->ifi_flags = dev->flags;
+ r->ifi_flags = dev_get_flags(dev);
r->ifi_change = change;
- if (!netif_running(dev) || !netif_carrier_ok(dev))
- r->ifi_flags &= ~IFF_RUNNING;
- else
- r->ifi_flags |= IFF_RUNNING;
-
RTA_PUT(skb, IFLA_IFNAME, strlen(dev->name)+1, dev->name);
+
+ if (1) {
+ u32 txqlen = dev->tx_queue_len;
+ RTA_PUT(skb, IFLA_TXQLEN, sizeof(txqlen), &txqlen);
+ }
+
+ if (1) {
+ struct ifmap map = {
+ .mem_start = dev->mem_start,
+ .mem_end = dev->mem_end,
+ .base_addr = dev->base_addr,
+ .irq = dev->irq,
+ .dma = dev->dma,
+ .port = dev->if_port,
+ };
+ RTA_PUT(skb, IFLA_MAP, sizeof(map), &map);
+ }
+
if (dev->addr_len) {
RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr);
RTA_PUT(skb, IFLA_BROADCAST, dev->addr_len, dev->broadcast);
}
+
if (1) {
- unsigned mtu = dev->mtu;
+ u32 mtu = dev->mtu;
RTA_PUT(skb, IFLA_MTU, sizeof(mtu), &mtu);
}
- if (dev->ifindex != dev->iflink)
- RTA_PUT(skb, IFLA_LINK, sizeof(int), &dev->iflink);
+
+ if (dev->ifindex != dev->iflink) {
+ u32 iflink = dev->iflink;
+ RTA_PUT(skb, IFLA_LINK, sizeof(iflink), &iflink);
+ }
+
if (dev->qdisc_sleeping)
RTA_PUT(skb, IFLA_QDISC,
strlen(dev->qdisc_sleeping->ops->id) + 1,
dev->qdisc_sleeping->ops->id);
- if (dev->master)
- RTA_PUT(skb, IFLA_MASTER, sizeof(int), &dev->master->ifindex);
+
+ if (dev->master) {
+ u32 master = dev->master->ifindex;
+ RTA_PUT(skb, IFLA_MASTER, sizeof(master), &master);
+ }
+
if (dev->get_stats) {
unsigned long *stats = (unsigned long*)dev->get_stats(dev);
if (stats) {
@@ -246,6 +268,30 @@
err = -EINVAL;
+ if (ifm->ifi_flags)
+ dev_change_flags(dev, ifm->ifi_flags);
+
+ if (ida[IFLA_MAP - 1]) {
+ if (!dev->set_config) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
+
+ if (!netif_device_present(dev)) {
+ err = -ENODEV;
+ goto out;
+ }
+
+ if (ida[IFLA_MAP - 1]->rta_len != RTA_LENGTH(sizeof(struct ifmap)))
+ goto out;
+
+ err = dev->set_config(dev, (struct ifmap *)
+ RTA_DATA(ida[IFLA_MAP - 1]));
+
+ if (err)
+ goto out;
+ }
+
if (ida[IFLA_ADDRESS - 1]) {
if (!dev->set_mac_address) {
err = -EOPNOTSUPP;
@@ -268,6 +314,23 @@
goto out;
memcpy(dev->broadcast, RTA_DATA(ida[IFLA_BROADCAST - 1]),
dev->addr_len);
+ }
+
+ if (ida[IFLA_MTU - 1]) {
+ if (ida[IFLA_MTU - 1]->rta_len != RTA_LENGTH(sizeof(u32)))
+ goto out;
+ err = dev_set_mtu(dev, *((u32 *) RTA_DATA(ida[IFLA_MTU - 1])));
+
+ if (err)
+ goto out;
+
+ }
+
+ if (ida[IFLA_TXQLEN - 1]) {
+ if (ida[IFLA_TXQLEN - 1]->rta_len != RTA_LENGTH(sizeof(u32)))
+ goto out;
+
+ dev->tx_queue_len = *((u32 *) RTA_DATA(ida[IFLA_TXQLEN - 1]));
}
err = 0;
--- 1.33/include/linux/rtnetlink.h Mon Apr 5 23:41:40 2004
+++ 1.35/include/linux/rtnetlink.h Tue Aug 24 20:06:53 2004
@@ -540,6 +540,8 @@
IFLA_QDISC,
IFLA_STATS,
IFLA_COST,
+ IFLA_TXQLEN,
+ IFLA_MAP,
#define IFLA_COST IFLA_COST
IFLA_PRIORITY,
#define IFLA_PRIORITY IFLA_PRIORITY
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 17:32 ` David S. Miller
@ 2004-08-24 18:15 ` Thomas Graf
2004-08-24 20:47 ` [RFC/PATCH] Convert architecture dependent rtnetlink attribute to fixed sized ones Thomas Graf
1 sibling, 0 replies; 13+ messages in thread
From: Thomas Graf @ 2004-08-24 18:15 UTC (permalink / raw)
To: David S. Miller; +Cc: herbert, kuznet, hadi, netdev
* David S. Miller <20040824103205.1ea9c999.davem@redhat.com> 2004-08-24 10:32
> On Tue, 24 Aug 2004 14:04:42 +0200
> Thomas Graf <tgraf@suug.ch> wrote:
>
> > * Herbert Xu <E1BzYMJ-000208-00@gondolin.me.apana.org.au> 2004-08-24 20:16
> > > Please think about the meaning of the value. Is anyone going to have a queue
> > > bigger than 2^32?
> > >
> > > And if the answer is yes, then please use u64.
> >
> > Can you please explain the actual reason for using a fixed width
> > type when all existing numeric attributes use arch depedent types?
>
> Because otherwise things will break and explode for 32-bit
> binaries running on 64-bit kernels, which is the situation
> for the majority of userland on some platforms.
> Also, we eventually want to be able to transmit netlink
> messages over real networks to remote clients.
Which would be no problem as the size of the attribute is
available to userspace.
> Therefore, like any other real network protocol, we should
> use fixed sized types in the data.
I'm fine with that, as long as it's done the same way for all
attributes.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC/PATCH] Convert architecture dependent rtnetlink attribute to fixed sized ones
2004-08-24 17:32 ` David S. Miller
2004-08-24 18:15 ` Thomas Graf
@ 2004-08-24 20:47 ` Thomas Graf
2004-08-24 22:44 ` David S. Miller
1 sibling, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2004-08-24 20:47 UTC (permalink / raw)
To: David S. Miller; +Cc: herbert, kuznet, hadi, netdev
* David S. Miller <20040824103205.1ea9c999.davem@redhat.com> 2004-08-24 10:32
> Because otherwise things will break and explode for 32-bit
> binaries running on 64-bit kernels, which is the situation
> for the majority of userland on some platforms.
Below is a patch which fixes some of the obvious architecture
dependent and easy to fix attributes. But there are dozens more of
them such as the int in struct tcf_police.
Is it really worth changing all of them and probably a few
userspace applications just to make it easier for them because
they won't have to check size for some of the attributes
anymore? On top of that, most of the ...->rta_len !=
RTA_LENGTH(struct XXX) checks in RTM_* handlers will fail anyway
if there is a int-size mismatch between kernel and userspace.
--- 1.17/net/sched/sch_atm.c Sat Feb 21 03:48:58 2004
+++ 1.18/net/sched/sch_atm.c Tue Aug 24 22:19:23 2004
@@ -639,7 +639,7 @@
RTA_PUT(skb,TCA_ATM_HDR,flow->hdr_len,flow->hdr);
if (flow->vcc) {
struct sockaddr_atmpvc pvc;
- int state;
+ u32 state;
pvc.sap_family = AF_ATMPVC;
pvc.sap_addr.itf = flow->vcc->dev ? flow->vcc->dev->number : -1;
--- 1.4/net/sched/police.c Sun Sep 28 19:00:40 2003
+++ 1.5/net/sched/police.c Tue Aug 24 22:19:23 2004
@@ -237,8 +237,10 @@
else
memset(&opt.peakrate, 0, sizeof(opt.peakrate));
RTA_PUT(skb, TCA_POLICE_TBF, sizeof(opt), &opt);
- if (p->result)
- RTA_PUT(skb, TCA_POLICE_RESULT, sizeof(int), &p->result);
+ if (p->result) {
+ u32 result = p->result;
+ RTA_PUT(skb, TCA_POLICE_RESULT, sizeof(u32), &result);
+ }
#ifdef CONFIG_NET_ESTIMATOR
if (p->ewma_rate)
RTA_PUT(skb, TCA_POLICE_AVRATE, 4, &p->ewma_rate);
--- 1.7/net/sched/cls_tcindex.c Sat Feb 21 03:37:25 2004
+++ 1.8/net/sched/cls_tcindex.c Tue Aug 24 22:19:23 2004
@@ -432,12 +432,15 @@
rta = (struct rtattr *) b;
RTA_PUT(skb,TCA_OPTIONS,0,NULL);
if (!fh) {
+ u32 hash = p->hash;
+ u32 shift = p->shift;
+ u32 fall_through = p->fall_through;
t->tcm_handle = ~0; /* whatever ... */
- RTA_PUT(skb,TCA_TCINDEX_HASH,sizeof(p->hash),&p->hash);
+ RTA_PUT(skb,TCA_TCINDEX_HASH,sizeof(u32),&hash);
RTA_PUT(skb,TCA_TCINDEX_MASK,sizeof(p->mask),&p->mask);
- RTA_PUT(skb,TCA_TCINDEX_SHIFT,sizeof(p->shift),&p->shift);
- RTA_PUT(skb,TCA_TCINDEX_FALL_THROUGH,sizeof(p->fall_through),
- &p->fall_through);
+ RTA_PUT(skb,TCA_TCINDEX_SHIFT,sizeof(u32),&shift);
+ RTA_PUT(skb,TCA_TCINDEX_FALL_THROUGH,sizeof(u32),
+ &fall_through);
} else {
if (p->perfect) {
t->tcm_handle = r-p->perfect;
--- 1.7/net/sched/cls_route.c Sat Feb 21 03:37:25 2004
+++ 1.8/net/sched/cls_route.c Tue Aug 24 22:19:23 2004
@@ -567,8 +567,10 @@
RTA_PUT(skb, TCA_ROUTE4_TO, sizeof(id), &id);
}
if (f->handle&0x80000000) {
- if ((f->handle>>16) != 0xFFFF)
- RTA_PUT(skb, TCA_ROUTE4_IIF, sizeof(f->iif), &f->iif);
+ if ((f->handle>>16) != 0xFFFF) {
+ u32 iif = f->iif;
+ RTA_PUT(skb, TCA_ROUTE4_IIF, sizeof(u32), &iif);
+ }
} else {
id = f->id>>16;
RTA_PUT(skb, TCA_ROUTE4_FROM, sizeof(id), &id);
--- 1.67/net/ipv6/route.c Tue Feb 24 00:11:22 2004
+++ 1.68/net/ipv6/route.c Tue Aug 24 22:19:23 2004
@@ -1532,8 +1532,10 @@
goto rtattr_failure;
if (rt->u.dst.neighbour)
RTA_PUT(skb, RTA_GATEWAY, 16, &rt->u.dst.neighbour->primary_key);
- if (rt->u.dst.dev)
- RTA_PUT(skb, RTA_OIF, sizeof(int), &rt->rt6i_dev->ifindex);
+ if (rt->u.dst.dev) {
+ u32 oif = rt->rt6i_dev->ifindex;
+ RTA_PUT(skb, RTA_OIF, sizeof(u32), &oif);
+ }
RTA_PUT(skb, RTA_PRIORITY, 4, &rt->rt6i_metric);
ci.rta_lastuse = jiffies_to_clock_t(jiffies - rt->u.dst.lastuse);
if (rt->rt6i_expires)
--- 1.98/net/ipv6/addrconf.c Mon Apr 5 23:41:40 2004
+++ 1.99/net/ipv6/addrconf.c Tue Aug 24 22:19:23 2004
@@ -2819,8 +2819,10 @@
RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr);
RTA_PUT(skb, IFLA_MTU, sizeof(mtu), &mtu);
- if (dev->ifindex != dev->iflink)
- RTA_PUT(skb, IFLA_LINK, sizeof(int), &dev->iflink);
+ if (dev->ifindex != dev->iflink) {
+ u32 link = dev->iflink;
+ RTA_PUT(skb, IFLA_LINK, sizeof(u32), &link);
+ }
subattr = (struct rtattr*)skb->tail;
--- 1.80/net/ipv4/route.c Fri Apr 30 01:26:35 2004
+++ 1.81/net/ipv4/route.c Tue Aug 24 22:19:23 2004
@@ -2291,8 +2291,10 @@
r->rtm_src_len = 32;
RTA_PUT(skb, RTA_SRC, 4, &rt->fl.fl4_src);
}
- if (rt->u.dst.dev)
- RTA_PUT(skb, RTA_OIF, sizeof(int), &rt->u.dst.dev->ifindex);
+ if (rt->u.dst.dev) {
+ u32 oif = rt->u.dst.dev->ifindex;
+ RTA_PUT(skb, RTA_OIF, 4, &oif);
+ }
#ifdef CONFIG_NET_CLS_ROUTE
if (rt->u.dst.tclassid)
RTA_PUT(skb, RTA_FLOW, 4, &rt->u.dst.tclassid);
@@ -2345,7 +2347,10 @@
}
} else
#endif
- RTA_PUT(skb, RTA_IIF, sizeof(int), &rt->fl.iif);
+ if (1) {
+ u32 iif = rt->fl.iif;
+ RTA_PUT(skb, RTA_IIF, sizeof(u32), &iif);
+ }
}
nlh->nlmsg_len = skb->tail - b;
--- 1.6/net/decnet/dn_table.c Tue May 6 09:58:43 2003
+++ 1.7/net/decnet/dn_table.c Tue Aug 24 22:19:23 2004
@@ -296,8 +296,10 @@
if (fi->fib_nhs == 1) {
if (fi->fib_nh->nh_gw)
RTA_PUT(skb, RTA_GATEWAY, 2, &fi->fib_nh->nh_gw);
- if (fi->fib_nh->nh_oif)
- RTA_PUT(skb, RTA_OIF, sizeof(int), &fi->fib_nh->nh_oif);
+ if (fi->fib_nh->nh_oif) {
+ u32 oif = fi->fib_nh->nh_oif;
+ RTA_PUT(skb, RTA_OIF, sizeof(u32), &oif);
+ }
}
if (fi->fib_nhs > 1) {
struct rtnexthop *nhp;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC/PATCH] Convert architecture dependent rtnetlink attribute to fixed sized ones
2004-08-24 20:47 ` [RFC/PATCH] Convert architecture dependent rtnetlink attribute to fixed sized ones Thomas Graf
@ 2004-08-24 22:44 ` David S. Miller
0 siblings, 0 replies; 13+ messages in thread
From: David S. Miller @ 2004-08-24 22:44 UTC (permalink / raw)
To: Thomas Graf; +Cc: herbert, kuznet, hadi, netdev
On Tue, 24 Aug 2004 22:47:04 +0200
Thomas Graf <tgraf@suug.ch> wrote:
> Below is a patch which fixes some of the obvious architecture
> dependent and easy to fix attributes. But there are dozens more of
> them such as the int in struct tcf_police.
'int' happens to be fine, since it is the same size on 32-bit vs.
64-bit systems. The conversion to explicitly sized types is
a nice cleanup, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK
2004-08-24 18:08 ` Thomas Graf
@ 2004-08-25 0:34 ` David S. Miller
0 siblings, 0 replies; 13+ messages in thread
From: David S. Miller @ 2004-08-25 0:34 UTC (permalink / raw)
To: Thomas Graf; +Cc: herbert, kuznet, hadi, netdev
On Tue, 24 Aug 2004 20:08:25 +0200
Thomas Graf <tgraf@suug.ch> wrote:
> --- 1.33/include/linux/rtnetlink.h Mon Apr 5 23:41:40 2004
> +++ 1.35/include/linux/rtnetlink.h Tue Aug 24 20:06:53 2004
> @@ -540,6 +540,8 @@
> IFLA_QDISC,
> IFLA_STATS,
> IFLA_COST,
> + IFLA_TXQLEN,
> + IFLA_MAP,
> #define IFLA_COST IFLA_COST
> IFLA_PRIORITY,
> #define IFLA_PRIORITY IFLA_PRIORITY
You cannot add new IFLA_* numbers to the middle of the
enumeration, that changes the subsequent existing
IFLA_* values which are hard-coded in userspace applications.
I've fixed this and applied your patch, thanks Thomas.
Could you please provide a 2.4.x version?
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2004-08-25 0:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-08-23 20:51 [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK Thomas Graf
2004-08-24 2:26 ` Herbert Xu
2004-08-24 9:49 ` Thomas Graf
2004-08-24 10:16 ` Herbert Xu
2004-08-24 12:04 ` Thomas Graf
2004-08-24 17:32 ` David S. Miller
2004-08-24 18:15 ` Thomas Graf
2004-08-24 20:47 ` [RFC/PATCH] Convert architecture dependent rtnetlink attribute to fixed sized ones Thomas Graf
2004-08-24 22:44 ` David S. Miller
2004-08-24 16:25 ` [PATCH] allow setting mtu and txqlen via RTM_SETLINK and provide txqlen via RTM_GETLINK Thomas Graf
2004-08-24 17:32 ` David S. Miller
2004-08-24 18:08 ` Thomas Graf
2004-08-25 0:34 ` David S. Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).