* Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-13 14:41 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: virtualization, netdev, Ira W. Snyder, linux-kernel, kvm
In-Reply-To: <200908131548.35199.arnd@arndb.de>
On Thu, Aug 13, 2009 at 03:48:35PM +0200, Arnd Bergmann wrote:
> On Thursday 13 August 2009, Arnd Bergmann wrote:
> > Unfortunately, this also implies that you could no longer simply use the
> > packet socket interface as you do currently, as I realized only now.
> > This obviously has a significant impact on your user space interface.
>
> Also, if we do the copy in the transport, it definitely means that we
> can't get to zero-copy RX/TX from guest space any more. The current
> vhost_net driver doesn't do that yet, but could be extended in the
> same way that I'm hoping to do it for macvtap.
>
> Arnd <><
The best way to do this IMO would be to add zero copy support to raw
sockets, vhost will then get it basically for free.
--
MST
^ permalink raw reply
* Re: [RFC] ipv6: Change %pI6 format to output compacted addresses?
From: Jens Rosenboom @ 2009-08-13 14:39 UTC (permalink / raw)
To: Brian Haley; +Cc: Linux Network Developers
In-Reply-To: <4A836D6D.1040400@hp.com>
[-- Attachment #1: Type: text/plain, Size: 3213 bytes --]
On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote:
> Jens Rosenboom wrote:
> > Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001
> > which might be compacted to 2001:db8::1. The code to do this could be
> > adapted from inet_ntop in glibc, which would add about 80 lines to
> > lib/vsprintf.c. How do you guys value the tradeoff between more readable
> > logging and increased kernel size?
> >
> > This was already mentioned in
> > http://kerneltrap.org/mailarchive/linux-netdev/2008/11/25/4231684 but
> > noone seems to have taken up on it.
>
> I think if any changes are made they should try and follow:
>
> http://www.ietf.org/id/draft-kawamura-ipv6-text-representation-03.txt
>
> For one thing, the code today doesn't print things like the v4-mapped
> address correctly.
>
> Anyways, can you try this patch, it's less than 40 new lines :)
> It might be good enough, but could probably use some help.
For a start, it didn't even compile. ;-) Here is a new version that also
fixes
- Leave %pi6 alone
- Don't compress a single :0:
- Do output 0
The results and also the remaining issues can be seen with the attached
test program, that also exposes a bug in glibc for v4-mapped addresses
from 0/16.
To fully conform to the cited draft, we would still have to implement
v4-mapped and also check whether a second run of zeros would be longer
than the first one, although the draft also suggests that operators
should avoid using this kind of addresses, so maybe this second issue
can be neglected.
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 756ccaf..5710c65 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -652,13 +652,53 @@ static char *ip6_addr_string(char *buf, char *end,
u8 *addr,
{
char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing
zero */
char *p = ip6_addr;
- int i;
+ int i, needcolon = 0, printhi;
+ u16 *addr16 = (u16 *)addr;
+ enum { DC_START, DC_MIDDLE, DC_DONE } colon = DC_START;
+
+ /* omit leading zeros and shorten using "::" */
- for (i = 0; i < 8; i++) {
- p = pack_hex_byte(p, addr[2 * i]);
- p = pack_hex_byte(p, addr[2 * i + 1]);
- if (!(spec.flags & SPECIAL) && i != 7)
+ if (!(spec.flags & SPECIAL)) {
+ for (i = 0; i < 8; i++) {
+ if (addr16[i] == 0 && addr16[i+1] == 0 && colon == DC_START) {
+ colon = DC_MIDDLE;
+ continue;
+ }
+ if (colon == DC_MIDDLE) {
+ if (addr16[i] == 0)
+ continue;
+ colon = DC_DONE;
+ *p++ = ':';
+ *p++ = ':';
+ } else if (needcolon)
+ *p++ = ':';
+ printhi = 0;
+ if (addr[2 * i]) {
+ if (addr[2 * i] > 0x0f)
+ p = pack_hex_byte(p, addr[2 * i]);
+ else
+ *p++ = hex_asc_lo(addr[2 * i]);
+ printhi++;
+ }
+ /*
+ * If we printed the high-order bits we must print the
+ * low-order ones, even if they're all zeros.
+ */
+ if (printhi || addr[2 * i + 1] > 0x0f)
+ p = pack_hex_byte(p, addr[2 * i + 1]);
+ else
+ *p++ = hex_asc_lo(addr[2 * i + 1]);
+ needcolon++;
+ }
+ if (colon == DC_MIDDLE) {
*p++ = ':';
+ *p++ = ':';
+ }
+ } else {
+ for (i = 0; i < 8; i++) {
+ p = pack_hex_byte(p, addr[2 * i]);
+ p = pack_hex_byte(p, addr[2 * i + 1]);
+ }
}
*p = '\0';
spec.flags &= ~SPECIAL;
[-- Attachment #2: test.c --]
[-- Type: text/x-csrc, Size: 867 bytes --]
/* Dirty test prog for %pI6 formatting */
/* Compile with: cc -o test test.c lib/vsprintf.o lib/ctype.o */
#include <arpa/inet.h>
extern int printf(const char *fmt, ...);
extern int sprintf(char * buf, const char *fmt, ...);
const char hex_asc[] = "0123456789abcdef";
sprint_symbol(void) { return 0; }
kallsyms_lookup(void) { return 0; }
warn_slowpath_null(void) { return 0;}
void dotest(void *addr) {
char res[500], res2[500];
inet_ntop(AF_INET6, addr, res2, 500);
sprintf(res, "%pi6 %pI6", addr, addr);
printf("%s %s\n", res, res2);
}
main() {
unsigned int addr[8];
int i;
for(i=0;i<8;i++) addr[i]=0;
dotest(addr);
addr[3]=htonl(0x100);
dotest(addr);
addr[3]=htonl(0x10000);
dotest(addr);
addr[0]=htonl(0x20010db8);
addr[1]=htonl(0x234);
dotest(addr);
addr[0]=htonl(0x20010000);
dotest(addr);
addr[3]=htonl(0xa);
dotest(addr);
}
^ permalink raw reply related
* Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-13 14:39 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: virtualization, Ira W. Snyder, netdev, kvm, linux-kernel
In-Reply-To: <200908131538.44465.arnd@arndb.de>
On Thu, Aug 13, 2009 at 03:38:43PM +0200, Arnd Bergmann wrote:
> On Thursday 13 August 2009, Michael S. Tsirkin wrote:
> > On Wed, Aug 12, 2009 at 07:59:47PM +0200, Arnd Bergmann wrote:
> > > The trick is to swap the virtqueues instead. virtio-net is actually
> > > mostly symmetric in just the same way that the physical wires on a
> > > twisted pair ethernet are symmetric (I like how that analogy fits).
> >
> > You need to really squint hard for it to look symmetric.
> >
> > For example, for RX, virtio allocates an skb, puts a descriptor on a
> > ring and waits for host to fill it in. Host system can not do the same:
> > guest does not have access to host memory.
> >
> > You can do a copy in transport to hide this fact, but it will kill
> > performance.
>
> Yes, that is what I was suggesting all along. The actual copy operation
> has to be done by the host transport, which is obviously different from
> the guest transport that just calls the host using vring_kick().
>
> Right now, the number of copy operations in your code is the same.
> You are doing the copy a little bit later in skb_copy_datagram_iovec(),
> which is indeed a very nice hack. Changing to a virtqueue based method
> would imply that the host needs to add each skb_frag_t to its outbound
> virtqueue, which then gets copied into the guests inbound virtqueue.
Which is a lot more code than just calling skb_copy_datagram_iovec.
> Unfortunately, this also implies that you could no longer simply use the
> packet socket interface as you do currently, as I realized only now.
> This obviously has a significant impact on your user space interface.
>
> Arnd <><
And, it will remove our ability to implement zero copy
down the road (when raw sockets support it).
--
MST
^ permalink raw reply
* Re: [RFC] ipv6: Change %pI6 format to output compacted addresses?
From: Jens Rosenboom @ 2009-08-13 14:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Linux Network Developers
In-Reply-To: <20090813141840.GA25935@infradead.org>
On Thu, 2009-08-13 at 10:18 -0400, Christoph Hellwig wrote:
> On Wed, Aug 12, 2009 at 05:39:20PM +0200, Jens Rosenboom wrote:
> > Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001
> > which might be compacted to 2001:db8::1. The code to do this could be
> > adapted from inet_ntop in glibc, which would add about 80 lines to
> > lib/vsprintf.c. How do you guys value the tradeoff between more readable
> > logging and increased kernel size?
>
> A little note: if you borrow code from glibc always make sure it's from
> an old enough version that is still GPLv2+ licensed.
The code for inet_ntop has as comment
* author:
* Paul Vixie, 1996.
while the whole file is
Copyright (c) 1996-1999 by Internet Software Consortium.
so it should probably be old enough. But it also doesn't look too nice
to me, so I'll try to go without it for now.
^ permalink raw reply
* Re: [RFC] ipv6: Change %pI6 format to output compacted addresses?
From: Christoph Hellwig @ 2009-08-13 14:18 UTC (permalink / raw)
To: Jens Rosenboom; +Cc: Linux Network Developers
In-Reply-To: <1250091560.6641.48.camel@fnki-nb00130>
On Wed, Aug 12, 2009 at 05:39:20PM +0200, Jens Rosenboom wrote:
> Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001
> which might be compacted to 2001:db8::1. The code to do this could be
> adapted from inet_ntop in glibc, which would add about 80 lines to
> lib/vsprintf.c. How do you guys value the tradeoff between more readable
> logging and increased kernel size?
A little note: if you borrow code from glibc always make sure it's from
an old enough version that is still GPLv2+ licensed.
^ permalink raw reply
* [PATCH net-next-2.6] bonding: wipe out printk's
From: Jiri Pirko @ 2009-08-13 14:11 UTC (permalink / raw)
To: davem; +Cc: fubar, bonding-devel, netdev
I did not introduce new lines over 80 chars. I even eliminated some of them.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index be799d2..cea5cfe 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1124,7 +1124,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
// detect loopback situation
if (!MAC_ADDRESS_COMPARE(&(lacpdu->actor_system), &(port->actor_system))) {
// INFO_RECEIVED_LOOPBACK_FRAMES
- printk(KERN_ERR DRV_NAME ": %s: An illegal loopback occurred on "
+ pr_err(DRV_NAME ": %s: An illegal loopback occurred on "
"adapter (%s). Check the configuration to verify that all "
"Adapters are connected to 802.3ad compliant switch ports\n",
port->slave->dev->master->name, port->slave->dev->name);
@@ -1306,11 +1306,13 @@ static void ad_port_selection_logic(struct port *port)
}
}
if (!curr_port) { // meaning: the port was related to an aggregator but was not on the aggregator port list
- printk(KERN_WARNING DRV_NAME ": %s: Warning: Port %d (on %s) was "
- "related to aggregator %d but was not on its port list\n",
- port->slave->dev->master->name,
- port->actor_port_number, port->slave->dev->name,
- port->aggregator->aggregator_identifier);
+ pr_warning(DRV_NAME ": %s: Warning: Port %d (on %s) "
+ "was related to aggregator %d but was not "
+ "on its port list\n",
+ port->slave->dev->master->name,
+ port->actor_port_number,
+ port->slave->dev->name,
+ port->aggregator->aggregator_identifier);
}
}
// search on all aggregators for a suitable aggregator for this port
@@ -1379,7 +1381,8 @@ static void ad_port_selection_logic(struct port *port)
pr_debug("Port %d joined LAG %d(new LAG)\n", port->actor_port_number, port->aggregator->aggregator_identifier);
} else {
- printk(KERN_ERR DRV_NAME ": %s: Port %d (on %s) did not find a suitable aggregator\n",
+ pr_err(DRV_NAME ": %s: Port %d (on %s) did not find "
+ "a suitable aggregator\n",
port->slave->dev->master->name,
port->actor_port_number, port->slave->dev->name);
}
@@ -1456,10 +1459,10 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
break;
default:
- printk(KERN_WARNING DRV_NAME
- ": %s: Impossible agg select mode %d\n",
- curr->slave->dev->master->name,
- __get_agg_selection_mode(curr->lag_ports));
+ pr_warning(DRV_NAME
+ ": %s: Impossible agg select mode %d\n",
+ curr->slave->dev->master->name,
+ __get_agg_selection_mode(curr->lag_ports));
break;
}
@@ -1562,7 +1565,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
// check if any partner replys
if (best->is_individual) {
- printk(KERN_WARNING DRV_NAME ": %s: Warning: No 802.3ad"
+ pr_warning(DRV_NAME ": %s: Warning: No 802.3ad"
" response from the link partner for any"
" adapters in the bond\n",
best->slave->dev->master->name);
@@ -1885,7 +1888,8 @@ int bond_3ad_bind_slave(struct slave *slave)
struct aggregator *aggregator;
if (bond == NULL) {
- printk(KERN_ERR DRV_NAME ": %s: The slave %s is not attached to its bond\n",
+ pr_err(DRV_NAME ": %s: The slave %s is not attached to "
+ "its bond\n",
slave->dev->master->name, slave->dev->name);
return -1;
}
@@ -1961,9 +1965,9 @@ void bond_3ad_unbind_slave(struct slave *slave)
// if slave is null, the whole port is not initialized
if (!port->slave) {
- printk(KERN_WARNING DRV_NAME ": Warning: %s: Trying to "
- "unbind an uninitialized port on %s\n",
- slave->dev->master->name, slave->dev->name);
+ pr_warning(DRV_NAME ": Warning: %s: Trying to "
+ "unbind an uninitialized port on %s\n",
+ slave->dev->master->name, slave->dev->name);
return;
}
@@ -1994,8 +1998,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
pr_debug("Some port(s) related to LAG %d - replaceing with LAG %d\n", aggregator->aggregator_identifier, new_aggregator->aggregator_identifier);
if ((new_aggregator->lag_ports == port) && new_aggregator->is_active) {
- printk(KERN_INFO DRV_NAME ": %s: Removing an active aggregator\n",
- aggregator->slave->dev->master->name);
+ pr_info(DRV_NAME ": %s: Removing an active aggregator\n",
+ aggregator->slave->dev->master->name);
// select new active aggregator
select_new_active_agg = 1;
}
@@ -2025,17 +2029,17 @@ void bond_3ad_unbind_slave(struct slave *slave)
ad_agg_selection_logic(__get_first_agg(port));
}
} else {
- printk(KERN_WARNING DRV_NAME ": %s: Warning: unbinding aggregator, "
- "and could not find a new aggregator for its ports\n",
- slave->dev->master->name);
+ pr_warning(DRV_NAME ": %s: Warning: unbinding aggregator, "
+ "and could not find a new aggregator for its ports\n",
+ slave->dev->master->name);
}
} else { // in case that the only port related to this aggregator is the one we want to remove
select_new_active_agg = aggregator->is_active;
// clear the aggregator
ad_clear_agg(aggregator);
if (select_new_active_agg) {
- printk(KERN_INFO DRV_NAME ": %s: Removing an active aggregator\n",
- slave->dev->master->name);
+ pr_info(DRV_NAME ": %s: Removing an active aggregator\n",
+ slave->dev->master->name);
// select new active aggregator
ad_agg_selection_logic(__get_first_agg(port));
}
@@ -2061,8 +2065,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
// clear the aggregator
ad_clear_agg(temp_aggregator);
if (select_new_active_agg) {
- printk(KERN_INFO DRV_NAME ": %s: Removing an active aggregator\n",
- slave->dev->master->name);
+ pr_info(DRV_NAME ": %s: Removing an active aggregator\n",
+ slave->dev->master->name);
// select new active aggregator
ad_agg_selection_logic(__get_first_agg(port));
}
@@ -2110,8 +2114,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
// select the active aggregator for the bond
if ((port = __get_first_port(bond))) {
if (!port->slave) {
- printk(KERN_WARNING DRV_NAME ": %s: Warning: bond's first port is "
- "uninitialized\n", bond->dev->name);
+ pr_warning(DRV_NAME ": %s: Warning: bond's first port is "
+ "uninitialized\n", bond->dev->name);
goto re_arm;
}
@@ -2124,8 +2128,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
// for each port run the state machines
for (port = __get_first_port(bond); port; port = __get_next_port(port)) {
if (!port->slave) {
- printk(KERN_WARNING DRV_NAME ": %s: Warning: Found an uninitialized "
- "port\n", bond->dev->name);
+ pr_warning(DRV_NAME ": %s: Warning: Found an uninitialized "
+ "port\n", bond->dev->name);
goto re_arm;
}
@@ -2166,8 +2170,9 @@ static void bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u
port = &(SLAVE_AD_INFO(slave).port);
if (!port->slave) {
- printk(KERN_WARNING DRV_NAME ": %s: Warning: port of slave %s is "
- "uninitialized\n", slave->dev->name, slave->dev->master->name);
+ pr_warning(DRV_NAME ": %s: Warning: port of slave %s "
+ "is uninitialized\n",
+ slave->dev->name, slave->dev->master->name);
return;
}
@@ -2212,9 +2217,9 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
// if slave is null, the whole port is not initialized
if (!port->slave) {
- printk(KERN_WARNING DRV_NAME ": Warning: %s: speed "
- "changed for uninitialized port on %s\n",
- slave->dev->master->name, slave->dev->name);
+ pr_warning(DRV_NAME ": Warning: %s: speed "
+ "changed for uninitialized port on %s\n",
+ slave->dev->master->name, slave->dev->name);
return;
}
@@ -2240,9 +2245,9 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
// if slave is null, the whole port is not initialized
if (!port->slave) {
- printk(KERN_WARNING DRV_NAME ": %s: Warning: duplex changed "
- "for uninitialized port on %s\n",
- slave->dev->master->name, slave->dev->name);
+ pr_warning(DRV_NAME ": %s: Warning: duplex changed "
+ "for uninitialized port on %s\n",
+ slave->dev->master->name, slave->dev->name);
return;
}
@@ -2269,9 +2274,9 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
// if slave is null, the whole port is not initialized
if (!port->slave) {
- printk(KERN_WARNING DRV_NAME ": Warning: %s: link status changed for "
- "uninitialized port on %s\n",
- slave->dev->master->name, slave->dev->name);
+ pr_warning(DRV_NAME ": Warning: %s: link status changed for "
+ "uninitialized port on %s\n",
+ slave->dev->master->name, slave->dev->name);
return;
}
@@ -2375,8 +2380,8 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
}
if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
- printk(KERN_DEBUG DRV_NAME ": %s: Error: "
- "bond_3ad_get_active_agg_info failed\n", dev->name);
+ pr_debug(DRV_NAME ": %s: Error: "
+ "bond_3ad_get_active_agg_info failed\n", dev->name);
goto out;
}
@@ -2385,9 +2390,8 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
if (slaves_in_agg == 0) {
/*the aggregator is empty*/
- printk(KERN_DEBUG DRV_NAME ": %s: Error: active "
- "aggregator is empty\n",
- dev->name);
+ pr_debug(DRV_NAME ": %s: Error: active aggregator is empty\n",
+ dev->name);
goto out;
}
@@ -2405,7 +2409,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
}
if (slave_agg_no >= 0) {
- printk(KERN_ERR DRV_NAME ": %s: Error: Couldn't find a slave to tx on "
+ pr_err(DRV_NAME ": %s: Error: Couldn't find a slave to tx on "
"for aggregator ID %d\n", dev->name, agg_id);
goto out;
}
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index bf45d20..2108706 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -194,7 +194,7 @@ static int tlb_initialize(struct bonding *bond)
new_hashtbl = kzalloc(size, GFP_KERNEL);
if (!new_hashtbl) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: Failed to allocate TLB hash table\n",
bond->dev->name);
return -1;
@@ -510,7 +510,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
client_info->slave->dev->dev_addr,
client_info->mac_dst);
if (!skb) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: failed to create an ARP packet\n",
client_info->slave->dev->master->name);
continue;
@@ -521,7 +521,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
if (client_info->tag) {
skb = vlan_put_tag(skb, client_info->vlan_id);
if (!skb) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: failed to insert VLAN tag\n",
client_info->slave->dev->master->name);
continue;
@@ -605,7 +605,7 @@ static void rlb_req_update_subnet_clients(struct bonding *bond, __be32 src_ip)
client_info = &(bond_info->rx_hashtbl[hash_index]);
if (!client_info->slave) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: found a client with no channel in "
"the client's hash table\n",
bond->dev->name);
@@ -802,7 +802,7 @@ static int rlb_initialize(struct bonding *bond)
new_hashtbl = kmalloc(size, GFP_KERNEL);
if (!new_hashtbl) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: Failed to allocate RLB hash table\n",
bond->dev->name);
return -1;
@@ -924,7 +924,7 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
skb = vlan_put_tag(skb, vlan->vlan_id);
if (!skb) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: failed to insert VLAN tag\n",
bond->dev->name);
continue;
@@ -954,7 +954,7 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[], int hw)
memcpy(s_addr.sa_data, addr, dev->addr_len);
s_addr.sa_family = dev->type;
if (dev_set_mac_address(dev, &s_addr)) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: dev_set_mac_address of dev %s failed! ALB "
"mode requires that the base driver support setting "
"the hw address also when the network device's "
@@ -1170,13 +1170,15 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
alb_set_slave_mac_addr(slave, free_mac_slave->perm_hwaddr,
bond->alb_info.rlb_enabled);
- printk(KERN_WARNING DRV_NAME
- ": %s: Warning: the hw address of slave %s is in use by "
- "the bond; giving it the hw address of %s\n",
- bond->dev->name, slave->dev->name, free_mac_slave->dev->name);
+ pr_warning(DRV_NAME
+ ": %s: Warning: the hw address of slave %s is "
+ "in use by the bond; giving it the hw address "
+ "of %s\n",
+ bond->dev->name, slave->dev->name,
+ free_mac_slave->dev->name);
} else if (has_bond_addr) {
- printk(KERN_ERR DRV_NAME
+ pr_err(DRV_NAME
": %s: Error: the hw address of slave %s is in use by the "
"bond; couldn't find a slave with a free hw address to "
"give it (this should not have happened)\n",
diff --git a/drivers/net/bonding/bond_ipv6.c b/drivers/net/bonding/bond_ipv6.c
index 0d73bf5..83921ab 100644
--- a/drivers/net/bonding/bond_ipv6.c
+++ b/drivers/net/bonding/bond_ipv6.c
@@ -79,14 +79,14 @@ static void bond_na_send(struct net_device *slave_dev,
ND_OPT_TARGET_LL_ADDR);
if (!skb) {
- printk(KERN_ERR DRV_NAME ": NA packet allocation failed\n");
+ pr_err(DRV_NAME ": NA packet allocation failed\n");
return;
}
if (vlan_id) {
skb = vlan_put_tag(skb, vlan_id);
if (!skb) {
- printk(KERN_ERR DRV_NAME ": failed to insert VLAN tag\n");
+ pr_err(DRV_NAME ": failed to insert VLAN tag\n");
return;
}
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3bf0cc6..4798d30 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4837,7 +4837,7 @@ static int bond_check_params(struct bond_params *params)
}
if (bond_mode == BOND_MODE_ALB) {
- printk(KERN_NOTICE DRV_NAME
+ pr_notice(DRV_NAME
": In ALB mode you might experience client "
"disconnections upon reconnection of a link if the "
"bonding module updelay parameter (%d msec) is "
@@ -4961,9 +4961,9 @@ static int bond_check_params(struct bond_params *params)
arp_ip_count);
for (i = 0; i < arp_ip_count; i++)
- printk(" %s", arp_ip_target[i]);
+ pr_info(" %s", arp_ip_target[i]);
- printk("\n");
+ pr_info("\n");
} else if (max_bonds) {
/* miimon and arp_interval not set, we need one so things
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 55bf34f..6044e12 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -218,9 +218,8 @@ static ssize_t bonding_store_slaves(struct device *d,
/* Quick sanity check -- is the bond interface up? */
if (!(bond->dev->flags & IFF_UP)) {
- printk(KERN_WARNING DRV_NAME
- ": %s: doing slave updates when interface is down.\n",
- bond->dev->name);
+ pr_warning(DRV_NAME ": %s: doing slave updates when "
+ "interface is down.\n", bond->dev->name);
}
/* Note: We can't hold bond->lock here, as bond_create grabs it. */
@@ -778,12 +777,14 @@ static ssize_t bonding_store_downdelay(struct device *d,
goto out;
} else {
if ((new_value % bond->params.miimon) != 0) {
- printk(KERN_WARNING DRV_NAME
- ": %s: Warning: down delay (%d) is not a multiple "
- "of miimon (%d), delay rounded to %d ms\n",
- bond->dev->name, new_value, bond->params.miimon,
- (new_value / bond->params.miimon) *
- bond->params.miimon);
+ pr_warning(DRV_NAME
+ ": %s: Warning: down delay (%d) is not a "
+ "multiple of miimon (%d), delay rounded "
+ "to %d ms\n",
+ bond->dev->name, new_value,
+ bond->params.miimon,
+ (new_value / bond->params.miimon) *
+ bond->params.miimon);
}
bond->params.downdelay = new_value / bond->params.miimon;
pr_info(DRV_NAME ": %s: Setting down delay to %d.\n",
@@ -838,12 +839,14 @@ static ssize_t bonding_store_updelay(struct device *d,
goto out;
} else {
if ((new_value % bond->params.miimon) != 0) {
- printk(KERN_WARNING DRV_NAME
- ": %s: Warning: up delay (%d) is not a multiple "
- "of miimon (%d), updelay rounded to %d ms\n",
- bond->dev->name, new_value, bond->params.miimon,
- (new_value / bond->params.miimon) *
- bond->params.miimon);
+ pr_warning(DRV_NAME
+ ": %s: Warning: up delay (%d) is not a "
+ "multiple of miimon (%d), updelay rounded "
+ "to %d ms\n",
+ bond->dev->name, new_value,
+ bond->params.miimon,
+ (new_value / bond->params.miimon) *
+ bond->params.miimon);
}
bond->params.updelay = new_value / bond->params.miimon;
pr_info(DRV_NAME ": %s: Setting up delay to %d.\n",
@@ -1299,9 +1302,9 @@ static ssize_t bonding_store_active_slave(struct device *d,
new_active = slave;
if (new_active == old_active) {
/* do nothing */
- printk(KERN_INFO DRV_NAME
- ": %s: %s is already the current active slave.\n",
- bond->dev->name, slave->dev->name);
+ pr_info(DRV_NAME
+ ": %s: %s is already the current active slave.\n",
+ bond->dev->name, slave->dev->name);
goto out;
}
else {
@@ -1309,17 +1312,17 @@ static ssize_t bonding_store_active_slave(struct device *d,
(old_active) &&
(new_active->link == BOND_LINK_UP) &&
IS_UP(new_active->dev)) {
- printk(KERN_INFO DRV_NAME
- ": %s: Setting %s as active slave.\n",
- bond->dev->name, slave->dev->name);
- bond_change_active_slave(bond, new_active);
+ pr_info(DRV_NAME
+ ": %s: Setting %s as active slave.\n",
+ bond->dev->name, slave->dev->name);
+ bond_change_active_slave(bond, new_active);
}
else {
- printk(KERN_INFO DRV_NAME
- ": %s: Could not set %s as active slave; "
- "either %s is down or the link is down.\n",
- bond->dev->name, slave->dev->name,
- slave->dev->name);
+ pr_info(DRV_NAME
+ ": %s: Could not set %s as active slave; "
+ "either %s is down or the link is down.\n",
+ bond->dev->name, slave->dev->name,
+ slave->dev->name);
}
goto out;
}
@@ -1537,8 +1540,8 @@ int bond_create_sysfs(void)
/* Is someone being kinky and naming a device bonding_master? */
if (__dev_get_by_name(&init_net,
class_attr_bonding_masters.attr.name))
- printk(KERN_ERR
- "network device named %s already exists in sysfs",
+ pr_err("network device named %s already "
+ "exists in sysfs",
class_attr_bonding_masters.attr.name);
ret = 0;
}
@@ -1566,7 +1569,7 @@ int bond_create_sysfs_entry(struct bonding *bond)
err = sysfs_create_group(&(dev->dev.kobj), &bonding_group);
if (err)
- printk(KERN_EMERG "eek! didn't create group!\n");
+ pr_emerg("eek! didn't create group!\n");
return err;
}
^ permalink raw reply related
* Re: [PATCH 2/3] security: introducing security_request_module
From: Serge E. Hallyn @ 2009-08-13 14:03 UTC (permalink / raw)
To: Eric Paris
Cc: linux-kernel, selinux, netdev, linux-security-module, sds, davem,
shemminger, kees, morgan, casey, dwalsh
In-Reply-To: <20090813134457.29186.7182.stgit@paris.rdu.redhat.com>
Quoting Eric Paris (eparis@redhat.com):
> Calling request_module() will trigger a userspace upcall which will load a
> new module into the kernel. This can be a dangerous event if the process
> able to trigger request_module() is able to control either the modprobe
> binary or the module binary. This patch adds a new security hook to
> request_module() which can be used by an LSM to control a processes ability
> to call request_module().
Is there a specific case in which you'd want to deny this ability
from a real task?
-serge
^ permalink raw reply
* Re: [PATCH] ipv6: Log the explicit address that triggered DAD failure
From: Brian Haley @ 2009-08-13 14:03 UTC (permalink / raw)
To: Jens Rosenboom; +Cc: Linux Network Developers, David Miller
In-Reply-To: <1250151364.6641.75.camel@fnki-nb00130>
Jens Rosenboom wrote:
> On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote:
> [...]
>> The other thing I've come across that is similar to this is the
>> issue that when DAD fails, /sbin/ip doesn't show that it did,
>> the address just stays in a tentative state forever:
>>
>> inet6 dead:beef::1/64 scope global tentative
>> valid_lft forever preferred_lft forever
>>
>> Does anyone have an issue of adding a "dadfailed" flag to make
>> this more obvious:
>>
>> inet6 dead:beef::1/64 scope global tentative dadfailed
>> valid_lft forever preferred_lft forever
>
> It looks like you would have to spend the last available bit in
> ifa_flags for that, not sure if that is worth it, how about setting it
> to tentative|deprecated instead?
Yes, I saw that it would be the last flag so I didn't know how that would
go over. My other thought was to define a new flags structure that can
be passed in/out like IFA_CACHEINFO is. It's a much larger patch...
> Some action should maybe also happen in the case that the address wasn't
> tentative anymore in ndisc_recv_na(). At least it should also log the
> address itself:
>
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 9eb68e9..1ba42bd 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -955,8 +955,8 @@ static void ndisc_recv_na(struct sk_buff *skb)
> */
> if (skb->pkt_type != PACKET_LOOPBACK)
> ND_PRINTK1(KERN_WARNING
> - "ICMPv6 NA: someone advertises our address on %s!\n",
> - ifp->idev->dev->name);
> + "ICMPv6 NA: someone advertises our address %pI6 on %s!\n",
> + &ifp->addr, ifp->idev->dev->name);
> in6_ifa_put(ifp);
> return;
> }
That looks good to me too, thanks.
-Brian
^ permalink raw reply
* Re: [PATCH 1/3] Networking: use CAP_NET_ADMIN when deciding to call request_module
From: Serge E. Hallyn @ 2009-08-13 14:01 UTC (permalink / raw)
To: Eric Paris
Cc: linux-kernel, selinux, netdev, linux-security-module, sds, davem,
shemminger, kees, morgan, casey, dwalsh
In-Reply-To: <20090813134451.29186.41664.stgit@paris.rdu.redhat.com>
Quoting Eric Paris (eparis@redhat.com):
> The networking code checks CAP_SYS_MODULE before using request_module() to
> try to load a kernel module. While this seems reasonable it's actually
> weakening system security since we have to allow CAP_SYS_MODULE for things
> like /sbin/ip and bluetoothd which need to be able to trigger module loads.
> CAP_SYS_MODULE actually grants those binaries the ability to directly load
> any code into the kernel. We should instead be protecting modprobe and the
> modules on disk, rather than granting random programs the ability to load code
> directly into the kernel. Instead we are going to gate those networking checks
> on CAP_NET_ADMIN which still limits them to root but which does not grant
> those processes the ability to load arbitrary code into the kernel.
Right, so we want to check that the caller has the rights to perform the
action which (in the end) requires the module load, not the module load
itself. CAP_SYS_MODULE should be reserved for callers which specify a
module (full pathname) to load.
> Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
> ---
>
> drivers/staging/comedi/comedi_fops.c | 8 ++++----
> net/core/dev.c | 2 +-
> net/ipv4/tcp_cong.c | 4 ++--
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
> index 42e4bc4..f54bb9b 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -1772,12 +1772,12 @@ static int comedi_open(struct inode *inode, struct file *file)
> mutex_lock(&dev->mutex);
> if (dev->attached)
> goto ok;
> - if (!capable(CAP_SYS_MODULE) && dev->in_request_module) {
> + if (!capable(CAP_NET_ADMIN) && dev->in_request_module) {
> DPRINTK("in request module\n");
> mutex_unlock(&dev->mutex);
> return -ENODEV;
> }
> - if (capable(CAP_SYS_MODULE) && dev->in_request_module)
> + if (capable(CAP_NET_ADMIN) && dev->in_request_module)
> goto ok;
>
> dev->in_request_module = 1;
> @@ -1790,8 +1790,8 @@ static int comedi_open(struct inode *inode, struct file *file)
>
> dev->in_request_module = 0;
>
> - if (!dev->attached && !capable(CAP_SYS_MODULE)) {
> - DPRINTK("not attached and not CAP_SYS_MODULE\n");
> + if (!dev->attached && !capable(CAP_NET_ADMIN)) {
> + DPRINTK("not attached and not CAP_NET_ADMIN\n");
> mutex_unlock(&dev->mutex);
> return -ENODEV;
> }
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 09fb03f..2604db9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1031,7 +1031,7 @@ void dev_load(struct net *net, const char *name)
> dev = __dev_get_by_name(net, name);
> read_unlock(&dev_base_lock);
>
> - if (!dev && capable(CAP_SYS_MODULE))
> + if (!dev && capable(CAP_NET_ADMIN))
> request_module("%s", name);
> }
>
> diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
> index e92beb9..6428b34 100644
> --- a/net/ipv4/tcp_cong.c
> +++ b/net/ipv4/tcp_cong.c
> @@ -116,7 +116,7 @@ int tcp_set_default_congestion_control(const char *name)
> spin_lock(&tcp_cong_list_lock);
> ca = tcp_ca_find(name);
> #ifdef CONFIG_MODULES
> - if (!ca && capable(CAP_SYS_MODULE)) {
> + if (!ca && capable(CAP_NET_ADMIN)) {
> spin_unlock(&tcp_cong_list_lock);
>
> request_module("tcp_%s", name);
> @@ -246,7 +246,7 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
>
> #ifdef CONFIG_MODULES
> /* not found attempt to autoload module */
> - if (!ca && capable(CAP_SYS_MODULE)) {
> + if (!ca && capable(CAP_NET_ADMIN)) {
> rcu_read_unlock();
> request_module("tcp_%s", name);
> rcu_read_lock();
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" 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
* Re: [net-next 11/36] bnx2x: MDC/MDIO CL45 IOCTLs
From: Ben Hutchings @ 2009-08-13 13:57 UTC (permalink / raw)
To: eilong; +Cc: David Miller, netdev
In-Reply-To: <1250101388.27379.163.camel@lb-tlvb-eilong>
On Wed, 2009-08-12 at 21:23 +0300, Eilon Greenstein wrote:
> As suggested by Ben Hutchings <bhutchings@solarflare.com>, using the MDC/MDIO
> IOCTL
>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> ---
[...]
> @@ -11419,6 +11458,14 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
> dev->vlan_features |= NETIF_F_TSO6;
> #endif
>
> + /* get_port_hwinfo() will set prtad and mmds properly */
> + bp->mdio.prtad = MDIO_PRTAD_NONE;
> + bp->mdio.mmds = 0;
I didn't see any code to change mmds.
> + bp->mdio.mode_support = MDIO_SUPPORTS_C45 | MDIO_EMULATE_C22;
MDIO_EMULATE_C22 means the mdio module will map a few clause 22
registers to clause 45 (and reject access to all other clause 22
registers), and you will never be passed devad = MDIO_DEVAD_NONE.
If you want to support clause 22 register access yourself then use
MDIO_SUPPORTS_C22.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC] ipv6: Change %pI6 format to output compacted addresses?
From: Jens Rosenboom @ 2009-08-13 13:52 UTC (permalink / raw)
To: Joe Perches; +Cc: Brian Haley, Linux Network Developers
In-Reply-To: <1250159961.28285.35.camel@Joe-Laptop.home>
On Thu, 2009-08-13 at 03:39 -0700, Joe Perches wrote:
> On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote:
> > Jens Rosenboom wrote:
> > > Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001
> > > which might be compacted to 2001:db8::1. The code to do this could be
> > > adapted from inet_ntop in glibc, which would add about 80 lines to
> > > lib/vsprintf.c. How do you guys value the tradeoff between more readable
> > > logging and increased kernel size?
> > >
> > > This was already mentioned in
> > > http://kerneltrap.org/mailarchive/linux-netdev/2008/11/25/4231684 but
> > > noone seems to have taken up on it.
> >
> > Anyways, can you try this patch, it's less than 40 new lines :)
> > It might be good enough, but could probably use some help.
>
> You'll need to invent a new %p qualifier type to allow
> compressed representation. Your patch will change current uses
> with seq_<foo> output in net, which could break userspace.
Would it be possible to transform this to using %pi6, as most of teh
seq_* stuff already does? It doesn't make sense to shorten the
un-colon-ed version anyway, I'll send an updated version of Brian's
patch soon.
^ permalink raw reply
* Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
From: Arnd Bergmann @ 2009-08-13 13:48 UTC (permalink / raw)
To: virtualization
Cc: Michael S. Tsirkin, netdev, Ira W. Snyder, linux-kernel, kvm
In-Reply-To: <200908131538.44465.arnd@arndb.de>
On Thursday 13 August 2009, Arnd Bergmann wrote:
> Unfortunately, this also implies that you could no longer simply use the
> packet socket interface as you do currently, as I realized only now.
> This obviously has a significant impact on your user space interface.
Also, if we do the copy in the transport, it definitely means that we
can't get to zero-copy RX/TX from guest space any more. The current
vhost_net driver doesn't do that yet, but could be extended in the
same way that I'm hoping to do it for macvtap.
Arnd <><
^ permalink raw reply
* [PATCH 3/3] SELinux: add selinux_kernel_module_request
From: Eric Paris @ 2009-08-13 13:45 UTC (permalink / raw)
To: linux-kernel, selinux, netdev, linux-security-module
Cc: sds, davem, shemminger, kees, morgan, casey, dwalsh
In-Reply-To: <20090813134451.29186.41664.stgit@paris.rdu.redhat.com>
This patch adds a new selinux hook so SELinux can arbitrate if a given
process should be allowed to trigger a request for the kernel to try to
load a module. This is a different operation than a process trying to load
a module itself, which is already protected by CAP_SYS_MODULE.
Signed-off-by: Eric Paris <eparis@redhat.com>
---
security/selinux/hooks.c | 6 ++++++
security/selinux/include/av_perm_to_string.h | 1 +
security/selinux/include/av_permissions.h | 1 +
3 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 38afca9..b0d72f1 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3296,6 +3296,11 @@ static int selinux_kernel_create_files_as(struct cred *new, struct inode *inode)
return 0;
}
+static int selinux_kernel_module_request(void)
+{
+ return task_has_system(current, SYSTEM__MODULE_REQUEST);
+}
+
static int selinux_task_setpgid(struct task_struct *p, pid_t pgid)
{
return current_has_perm(p, PROCESS__SETPGID);
@@ -5457,6 +5462,7 @@ static struct security_operations selinux_ops = {
.cred_prepare = selinux_cred_prepare,
.kernel_act_as = selinux_kernel_act_as,
.kernel_create_files_as = selinux_kernel_create_files_as,
+ .kernel_module_request = selinux_kernel_module_request,
.task_setpgid = selinux_task_setpgid,
.task_getpgid = selinux_task_getpgid,
.task_getsid = selinux_task_getsid,
diff --git a/security/selinux/include/av_perm_to_string.h b/security/selinux/include/av_perm_to_string.h
index 31df1d7..2b683ad 100644
--- a/security/selinux/include/av_perm_to_string.h
+++ b/security/selinux/include/av_perm_to_string.h
@@ -107,6 +107,7 @@
S_(SECCLASS_SYSTEM, SYSTEM__SYSLOG_READ, "syslog_read")
S_(SECCLASS_SYSTEM, SYSTEM__SYSLOG_MOD, "syslog_mod")
S_(SECCLASS_SYSTEM, SYSTEM__SYSLOG_CONSOLE, "syslog_console")
+ S_(SECCLASS_SYSTEM, SYSTEM__MODULE_REQUEST, "module_request")
S_(SECCLASS_CAPABILITY, CAPABILITY__CHOWN, "chown")
S_(SECCLASS_CAPABILITY, CAPABILITY__DAC_OVERRIDE, "dac_override")
S_(SECCLASS_CAPABILITY, CAPABILITY__DAC_READ_SEARCH, "dac_read_search")
diff --git a/security/selinux/include/av_permissions.h b/security/selinux/include/av_permissions.h
index 0b41ad5..0546d61 100644
--- a/security/selinux/include/av_permissions.h
+++ b/security/selinux/include/av_permissions.h
@@ -530,6 +530,7 @@
#define SYSTEM__SYSLOG_READ 0x00000002UL
#define SYSTEM__SYSLOG_MOD 0x00000004UL
#define SYSTEM__SYSLOG_CONSOLE 0x00000008UL
+#define SYSTEM__MODULE_REQUEST 0x00000010UL
#define CAPABILITY__CHOWN 0x00000001UL
#define CAPABILITY__DAC_OVERRIDE 0x00000002UL
#define CAPABILITY__DAC_READ_SEARCH 0x00000004UL
^ permalink raw reply related
* Re: 8139cp dma-debug warning.
From: Dave Jones @ 2009-08-13 13:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Francois Romieu
In-Reply-To: <20090812.222046.60033759.davem@davemloft.net>
On Wed, Aug 12, 2009 at 10:20:46PM -0700, David Miller wrote:
> From: Dave Jones <davej@redhat.com>
> Date: Wed, 12 Aug 2009 13:13:33 -0400
>
> > There's another instance of the same further in the file.
> >
> > Does this look right?
>
> Dave, Francois posted the following fix to lkml (he forgot
> to CC: netdev and the people reporting this bug, oh well)
Ah, I missed it (disk crashed on friday, so I've been backlogged
since then). Thanks for the forward.
> Did you see it? It should fix the bug.
>
> 8139cp: balance dma_map_single vs dma_unmap_single pair
>
> The driver always:
> 1. allocate cp->rx_buf_sz + NET_IP_ALIGN
> 2. map cp->rx_buf_sz
>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> drivers/net/8139cp.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
> index 50efde1..d0dbbf3 100644
> --- a/drivers/net/8139cp.c
> +++ b/drivers/net/8139cp.c
> @@ -515,7 +515,7 @@ rx_status_loop:
> dma_addr_t mapping;
> struct sk_buff *skb, *new_skb;
> struct cp_desc *desc;
> - unsigned buflen;
> + const unsigned buflen = cp->rx_buf_sz;
>
> skb = cp->rx_skb[rx_tail];
> BUG_ON(!skb);
> @@ -549,8 +549,7 @@ rx_status_loop:
> pr_debug("%s: rx slot %d status 0x%x len %d\n",
> dev->name, rx_tail, status, len);
>
> - buflen = cp->rx_buf_sz + NET_IP_ALIGN;
> - new_skb = netdev_alloc_skb(dev, buflen);
> + new_skb = netdev_alloc_skb(dev, buflen + NET_IP_ALIGN);
> if (!new_skb) {
> dev->stats.rx_dropped++;
> goto rx_next;
Below this, we're still doing an skb_reserve(NET_IP_ALIGN) on new_skb.
Although the mapping is now constantly sized, aren't we still wastefully
bumping the data/tail of the skb twice ?
Dave
^ permalink raw reply
* Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
From: Arnd Bergmann @ 2009-08-13 13:45 UTC (permalink / raw)
To: Anthony Liguori
Cc: Michael S. Tsirkin, virtualization, Ira W. Snyder, netdev, kvm,
linux-kernel
In-Reply-To: <4A83167E.2080701@codemonkey.ws>
On Wednesday 12 August 2009, Anthony Liguori wrote:
> At any rate, I'd like to see performance results before we consider
> trying to reuse virtio code.
Yes, I agree. I'd also like to do more work on the macvlan extensions
to see if it works out without involving a socket. Passing the socket
into the vhost_net device is a nice feature of the current implementation
that we'd have to give up for something else (e.g. making the vhost
a real network interface that you can hook up to a bridge) if it were
to use virtio.
Unless I can come up with a solution that is clearly superior, I'm
taking back my objections on that part for now.
Arnd <><
^ permalink raw reply
* [PATCH 2/3] security: introducing security_request_module
From: Eric Paris @ 2009-08-13 13:44 UTC (permalink / raw)
To: linux-kernel, selinux, netdev, linux-security-module
Cc: sds, davem, shemminger, kees, morgan, casey, dwalsh
In-Reply-To: <20090813134451.29186.41664.stgit@paris.rdu.redhat.com>
Calling request_module() will trigger a userspace upcall which will load a
new module into the kernel. This can be a dangerous event if the process
able to trigger request_module() is able to control either the modprobe
binary or the module binary. This patch adds a new security hook to
request_module() which can be used by an LSM to control a processes ability
to call request_module().
Signed-off-by: Eric Paris <eparis@redhat.com>
---
include/linux/security.h | 10 ++++++++++
kernel/kmod.c | 4 ++++
security/capability.c | 6 ++++++
security/security.c | 5 +++++
4 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index d5f6578..34c5465 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -678,6 +678,9 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @inode points to the inode to use as a reference.
* The current task must be the one that nominated @inode.
* Return 0 if successful.
+ * @kernel_module_request:
+ * Ability to trigger the kernel to automatically upcall to userspace for
+ * userspace to load a kernel module with the given name.
* @task_setuid:
* Check permission before setting one or more of the user identity
* attributes of the current process. The @flags parameter indicates
@@ -1500,6 +1503,7 @@ struct security_operations {
void (*cred_commit)(struct cred *new, const struct cred *old);
int (*kernel_act_as)(struct cred *new, u32 secid);
int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
+ int (*kernel_module_request)(void);
int (*task_setuid) (uid_t id0, uid_t id1, uid_t id2, int flags);
int (*task_fix_setuid) (struct cred *new, const struct cred *old,
int flags);
@@ -1755,6 +1759,7 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
void security_commit_creds(struct cred *new, const struct cred *old);
int security_kernel_act_as(struct cred *new, u32 secid);
int security_kernel_create_files_as(struct cred *new, struct inode *inode);
+int security_kernel_module_request(void);
int security_task_setuid(uid_t id0, uid_t id1, uid_t id2, int flags);
int security_task_fix_setuid(struct cred *new, const struct cred *old,
int flags);
@@ -2306,6 +2311,11 @@ static inline int security_kernel_create_files_as(struct cred *cred,
return 0;
}
+static inline int security_kernel_module_request(void)
+{
+ return 0;
+}
+
static inline int security_task_setuid(uid_t id0, uid_t id1, uid_t id2,
int flags)
{
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 385c31a..5a7ae57 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -78,6 +78,10 @@ int __request_module(bool wait, const char *fmt, ...)
#define MAX_KMOD_CONCURRENT 50 /* Completely arbitrary value - KAO */
static int kmod_loop_msg;
+ ret = security_kernel_module_request();
+ if (ret)
+ return ret;
+
va_start(args, fmt);
ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args);
va_end(args);
diff --git a/security/capability.c b/security/capability.c
index 4f23f4f..06400cf 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -396,6 +396,11 @@ static int cap_kernel_create_files_as(struct cred *new, struct inode *inode)
return 0;
}
+static int cap_kernel_module_request(void)
+{
+ return 0;
+}
+
static int cap_task_setuid(uid_t id0, uid_t id1, uid_t id2, int flags)
{
return 0;
@@ -961,6 +966,7 @@ void security_fixup_ops(struct security_operations *ops)
set_to_cap_if_null(ops, cred_commit);
set_to_cap_if_null(ops, kernel_act_as);
set_to_cap_if_null(ops, kernel_create_files_as);
+ set_to_cap_if_null(ops, kernel_module_request);
set_to_cap_if_null(ops, task_setuid);
set_to_cap_if_null(ops, task_fix_setuid);
set_to_cap_if_null(ops, task_setgid);
diff --git a/security/security.c b/security/security.c
index b98c684..f88eaf6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -709,6 +709,11 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
return security_ops->kernel_create_files_as(new, inode);
}
+int security_kernel_module_request(void)
+{
+ return security_ops->kernel_module_request();
+}
+
int security_task_setuid(uid_t id0, uid_t id1, uid_t id2, int flags)
{
return security_ops->task_setuid(id0, id1, id2, flags);
^ permalink raw reply related
* [PATCH 1/3] Networking: use CAP_NET_ADMIN when deciding to call request_module
From: Eric Paris @ 2009-08-13 13:44 UTC (permalink / raw)
To: linux-kernel, selinux, netdev, linux-security-module
Cc: sds, davem, shemminger, kees, morgan, casey, dwalsh
The networking code checks CAP_SYS_MODULE before using request_module() to
try to load a kernel module. While this seems reasonable it's actually
weakening system security since we have to allow CAP_SYS_MODULE for things
like /sbin/ip and bluetoothd which need to be able to trigger module loads.
CAP_SYS_MODULE actually grants those binaries the ability to directly load
any code into the kernel. We should instead be protecting modprobe and the
modules on disk, rather than granting random programs the ability to load code
directly into the kernel. Instead we are going to gate those networking checks
on CAP_NET_ADMIN which still limits them to root but which does not grant
those processes the ability to load arbitrary code into the kernel.
Signed-off-by: Eric Paris <eparis@redhat.com>
---
drivers/staging/comedi/comedi_fops.c | 8 ++++----
net/core/dev.c | 2 +-
net/ipv4/tcp_cong.c | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index 42e4bc4..f54bb9b 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1772,12 +1772,12 @@ static int comedi_open(struct inode *inode, struct file *file)
mutex_lock(&dev->mutex);
if (dev->attached)
goto ok;
- if (!capable(CAP_SYS_MODULE) && dev->in_request_module) {
+ if (!capable(CAP_NET_ADMIN) && dev->in_request_module) {
DPRINTK("in request module\n");
mutex_unlock(&dev->mutex);
return -ENODEV;
}
- if (capable(CAP_SYS_MODULE) && dev->in_request_module)
+ if (capable(CAP_NET_ADMIN) && dev->in_request_module)
goto ok;
dev->in_request_module = 1;
@@ -1790,8 +1790,8 @@ static int comedi_open(struct inode *inode, struct file *file)
dev->in_request_module = 0;
- if (!dev->attached && !capable(CAP_SYS_MODULE)) {
- DPRINTK("not attached and not CAP_SYS_MODULE\n");
+ if (!dev->attached && !capable(CAP_NET_ADMIN)) {
+ DPRINTK("not attached and not CAP_NET_ADMIN\n");
mutex_unlock(&dev->mutex);
return -ENODEV;
}
diff --git a/net/core/dev.c b/net/core/dev.c
index 09fb03f..2604db9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1031,7 +1031,7 @@ void dev_load(struct net *net, const char *name)
dev = __dev_get_by_name(net, name);
read_unlock(&dev_base_lock);
- if (!dev && capable(CAP_SYS_MODULE))
+ if (!dev && capable(CAP_NET_ADMIN))
request_module("%s", name);
}
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index e92beb9..6428b34 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -116,7 +116,7 @@ int tcp_set_default_congestion_control(const char *name)
spin_lock(&tcp_cong_list_lock);
ca = tcp_ca_find(name);
#ifdef CONFIG_MODULES
- if (!ca && capable(CAP_SYS_MODULE)) {
+ if (!ca && capable(CAP_NET_ADMIN)) {
spin_unlock(&tcp_cong_list_lock);
request_module("tcp_%s", name);
@@ -246,7 +246,7 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
#ifdef CONFIG_MODULES
/* not found attempt to autoload module */
- if (!ca && capable(CAP_SYS_MODULE)) {
+ if (!ca && capable(CAP_NET_ADMIN)) {
rcu_read_unlock();
request_module("tcp_%s", name);
rcu_read_lock();
^ permalink raw reply related
* Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
From: Arnd Bergmann @ 2009-08-13 13:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtualization, Ira W. Snyder, netdev, kvm, linux-kernel
In-Reply-To: <20090813060615.GC3029@redhat.com>
On Thursday 13 August 2009, Michael S. Tsirkin wrote:
> On Wed, Aug 12, 2009 at 07:59:47PM +0200, Arnd Bergmann wrote:
> > The trick is to swap the virtqueues instead. virtio-net is actually
> > mostly symmetric in just the same way that the physical wires on a
> > twisted pair ethernet are symmetric (I like how that analogy fits).
>
> You need to really squint hard for it to look symmetric.
>
> For example, for RX, virtio allocates an skb, puts a descriptor on a
> ring and waits for host to fill it in. Host system can not do the same:
> guest does not have access to host memory.
>
> You can do a copy in transport to hide this fact, but it will kill
> performance.
Yes, that is what I was suggesting all along. The actual copy operation
has to be done by the host transport, which is obviously different from
the guest transport that just calls the host using vring_kick().
Right now, the number of copy operations in your code is the same.
You are doing the copy a little bit later in skb_copy_datagram_iovec(),
which is indeed a very nice hack. Changing to a virtqueue based method
would imply that the host needs to add each skb_frag_t to its outbound
virtqueue, which then gets copied into the guests inbound virtqueue.
Unfortunately, this also implies that you could no longer simply use the
packet socket interface as you do currently, as I realized only now.
This obviously has a significant impact on your user space interface.
Arnd <><
^ permalink raw reply
* Re: [PATCH] net/ipv4, linux-2.6.30.4
From: Arnd Hannemann @ 2009-08-13 12:40 UTC (permalink / raw)
To: David Miller; +Cc: slot.daniel@gmail.com, netdev@vger.kernel.org
In-Reply-To: <20090812.145549.228391386.davem@davemloft.net>
David Miller schrieb:
> From: Daniel Slot <slot.daniel@gmail.com>
> Date: Wed, 12 Aug 2009 20:47:44 +0200
>
>> RFC 4653 specifies Non-Congestion Robustness (NCR) for TCP.
>> In the absence of explicit congestion notification from the network, TCP
>> uses loss as an indication of congestion.
>> One of the ways TCP detects loss is using the arrival of three duplicate
>> acknowledgments.
>> However, this heuristic is not always correct,
>> notably in the case when network paths reorder segments (for whatever
>> reason), resulting in degraded performance.
>
> Linux's TCP stack already has sophisticated reordering detection.ä
Hmm, sophisticated? Sorry, it seemed pretty rudimental/random to me.
Firstly, tp->reordering never shrinks for a given connection
unless an RTO occurs. If that happens tp->reordering is reset to sysctl_tcp_reordering
(but it was initialized with a potentially different value from destination cache).
Why?
Secondly, it simply disables FACK? Disabling FACK completely may (or not) be
the correct solution, if reordering is present. But why don't reenable FACK
after no more reordering is detected? It won't even get re-enabled if an RTO occurs.
It seems even more strange that tp->reordering is used in FACK paths, too.
So if one sets a high sysctl_tcp_reordering, because one expects reordering,
tcp_update_reordering will probably NOT disable FACK, but instead FACK will
be used with a high tp->reordering value.
Thirdly, in most cases it will only
trigger if spurious retransmits already happened. If it triggers in advance (due to
SACK logic, the updated reordering metric will be IMO one to small, leading again
to a spurios retransmit, if a reordering event with the same length will happen again)
IOW it will mostly only reduce the damage to congestion control, but will send out
spurious packets nevertheless.
In my point of view, on should at least build some EWMA or histogram, or build some
whatever statistcs to measure detected reordering and based on this measurement,
adjust the dupthresh (or max_burst, or whatever). Off course, there is always
the question of how much better such an sophisticated statistic will work,
than the current very pragmatic solution...
Please correct me if I'm wrong or just too stupid to understand this stuff.
(very likely;-)
>
>> TCP-NCR is designed to mitigate this degraded performance by increasing the
>> number of duplicate acknowledgments required to trigger loss recovery,
>> based on the current state of the connection, in an effort to better
>> disambiguate true segment loss from segment reordering.
>
> We already have code in the stack which tries to detect packet
> reordering with a high level of sophistication.
On the contrary RFC 4653 does not even try to detect reordering. It simply
delays the congestion response in a way which seems very straightforward.
Of course there is the negative impact of increased latency. (Loss recovery
takes longer). However, for large ftp/http transfers, who cares about latency?
There must be some logic in the kernel to detect applications which are
doing bulk transfers for the buffer autotuning, what about enabling RFC 4653
in case such an application is detected?
Daniel, I would assume RFC 4653 would simply work with FACK, at least if
there is no reordering present?
Best regards,
Arnd
--
Dipl.-Inform. Arnd Hannemann
RWTH Aachen University
Dept. of Computer Science, Informatik 4
Ahornstr. 55, D-52074 Aachen, Germany
Phone: (+49 241) 80-21423 Fax: (+49 241) 80-22220
^ permalink raw reply
* Re: AlacrityVM numbers updated for 31-rc4
From: Gregory Haskins @ 2009-08-13 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: alacrityvm-devel, alacrityvm-users, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org, Michael S. Tsirkin, netdev
In-Reply-To: <4A834E75.9060204@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
Gregory Haskins wrote:
> Anthony Liguori wrote:
>> Just FYI, the numbers quoted are wrong for virtio-u. Greg's machine
>> didn't have high res timers enabled in the kernel. He'll post newer
>> numbers later but they're much better than these (venet is still ahead
>> though).
>>
>> Regards,
>>
>> Anthony Liguori
>
> Anthony is correct. The new numbers after fixing the HRT clock issue are:
>
> virtio-u: 2670Mb/s, 266us rtt (3764 tps udp-rr)
>
> I will update the charts later tonight.
>
done
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]
^ permalink raw reply
* Re: [Iproute2]: failed replacing internal netem qdisc with pfifo
From: Jarek Poplawski @ 2009-08-13 12:03 UTC (permalink / raw)
To: Marcela Maslanova; +Cc: netdev, shemminger
In-Reply-To: <2036847082.914731250159200857.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com>
On 13-08-2009 12:26, Marcela Maslanova wrote:
> Hello,
> I've received bug report about not being replaced netem with pfifo
> anymore: https://bugzilla.redhat.com/show_bug.cgi?id=515522
> This should be possible according to documentation [1].
> Is this intentional change or is it a bug?
It's intentional:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=02201464119334690fe209849843881b8e9cfa9f
>
> It worked with kernel-2.6.27.5-117.fc10 and iproute-2.6.27. The problem
> is still present with current iproute and kernel-2.6.29.6-217.2.3.fc11.
Yes, netem is classless since 2.6.29, so this documentation needs
updating.
Best regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH] Speed-up pfifo_fast lookup using a bitmap
From: Jarek Poplawski @ 2009-08-13 11:27 UTC (permalink / raw)
To: Krishna Kumar2; +Cc: davem, herbert, kaber, netdev, netdev-owner
In-Reply-To: <OFD762AF41.E7A6EE8D-ON65257611.00389F3C-65257611.003AC5BB@in.ibm.com>
On Thu, Aug 13, 2009 at 04:11:57PM +0530, Krishna Kumar2 wrote:
> > Jarek Poplawski <jarkao2@gmail.com>
> >
> > > but the test numbers came a little less, since it takes a few more
> > > memory references on enqueue/dequeue.
> >
> > If it's exactly "a little less" I'd consider keeping it private yet...
>
> Sounds reasonable. To quantify that, I will test again for a longer
> run and report the difference.
Yes, more numbers would be appreciated.
>
> > Btw, I wonder how much gain of your previous (_CAN_BYPASS) patch is
> > saved after this change...
>
> The tests are on the latest tree which contains CAN_BYPASS. So a
> single netperf process running this change will get no advantage
> since this enqueue/dequeue never happens unless the NIC is slow.
> But for multiple processes, it should help.
I mean: since the previous patch saved ~2% on omitting enqueue/dequeue,
and now enqueue/dequeue is ~2% faster, is it still worth to omit this?
Regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH 0/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations
From: Neil Horman @ 2009-08-13 10:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev, rostedt
In-Reply-To: <20090812.221515.147337283.davem@davemloft.net>
On Wed, Aug 12, 2009 at 10:15:15PM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Fri, 7 Aug 2009 16:21:30 -0400
>
> > Tested by me, working well, applies against the head of the net-next tree
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
> For some reason they don't apply cleanly to net-next-2.6, probably
> because of the net-2.6 merge I did recently.
>
> Neil, could you please respin, and also could you please not
> use the same subject line for all 3 patches? That becomes the
> commit message header, and it should be unique for each patch
> since each patch does something different. :)
>
>
>
Sure, no problem, I'll respin them all today and repost.
Thanks!
Neil
^ permalink raw reply
* Re: [PATCH] Speed-up pfifo_fast lookup using a bitmap
From: Krishna Kumar2 @ 2009-08-13 10:41 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: davem, herbert, kaber, netdev, netdev-owner
In-Reply-To: <20090813100826.GA6042@ff.dom.local>
> Jarek Poplawski <jarkao2@gmail.com>
>
> > but the test numbers came a little less, since it takes a few more
> > memory references on enqueue/dequeue.
>
> If it's exactly "a little less" I'd consider keeping it private yet...
Sounds reasonable. To quantify that, I will test again for a longer
run and report the difference.
> Btw, I wonder how much gain of your previous (_CAN_BYPASS) patch is
> saved after this change...
The tests are on the latest tree which contains CAN_BYPASS. So a
single netperf process running this change will get no advantage
since this enqueue/dequeue never happens unless the NIC is slow.
But for multiple processes, it should help.
> - * Convert a bitmap to the first band number where an skb is queue'd,
where:
> + * Convert a bitmap to the first band number where an skb is queued,
where:
>
> > + * bitmap=0 means there are no skbs for any bands
> > + * bitmap=1 means there is a skb on band 0
>
> - * bitmap=1 means there is a skb on band 0
> + * bitmap=1 means there is an skb on band 0
>
> > + * bitmap=7 means there are skbs on all 3 bands, etc.
> > + */
> > +static const int bitmap2band[] =
> > + {-1, 0, 1, 0, 2, 0, 1, 0};
>
> Why wrapped?
cut-n-paste of prio2band, I will fix this and the above comments.
>
> ...
> ... pfifo_fast_reset(...)
> {
> ...
> + qdisc->bitmap = 0; ?
> }
Correct. Thanks for pointing this out.
Regards,
- KK
^ permalink raw reply
* Re: [RFC] ipv6: Change %pI6 format to output compacted addresses?
From: Joe Perches @ 2009-08-13 10:39 UTC (permalink / raw)
To: Brian Haley; +Cc: Jens Rosenboom, Linux Network Developers
In-Reply-To: <4A836D6D.1040400@hp.com>
On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote:
> Jens Rosenboom wrote:
> > Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001
> > which might be compacted to 2001:db8::1. The code to do this could be
> > adapted from inet_ntop in glibc, which would add about 80 lines to
> > lib/vsprintf.c. How do you guys value the tradeoff between more readable
> > logging and increased kernel size?
> >
> > This was already mentioned in
> > http://kerneltrap.org/mailarchive/linux-netdev/2008/11/25/4231684 but
> > noone seems to have taken up on it.
>
> Anyways, can you try this patch, it's less than 40 new lines :)
> It might be good enough, but could probably use some help.
You'll need to invent a new %p qualifier type to allow
compressed representation. Your patch will change current uses
with seq_<foo> output in net, which could break userspace.
^ 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