* [PATCH]: tipc: Fix oops on send prior to entering networked mode
From: Neil Horman @ 2010-02-19 19:40 UTC (permalink / raw)
To: netdev
Cc: per.liden, jon.maloy, allan.stephens, tipc-discussion, davem,
nhorman
Fix TIPC to disallow sending to remote addresses prior to entering NET_MODE
user programs can oops the kernel by sending datagrams via AF_TIPC prior to
entering networked mode. The following backtrace has been observed:
ID: 13459 TASK: ffff810014640040 CPU: 0 COMMAND: "tipc-client"
#0 [ffff81002d9a5810] crash_kexec at ffffffff800ac5b9
#1 [ffff81002d9a58d0] __die at ffffffff80065127
#2 [ffff81002d9a5910] do_page_fault at ffffffff80066da7
#3 [ffff81002d9a5a00] error_exit at ffffffff8005dde9
[exception RIP: tipc_node_select_next_hop+90]
RIP: ffffffff8869d3c3 RSP: ffff81002d9a5ab8 RFLAGS: 00010202
RAX: 0000000000000001 RBX: 0000000000000001 RCX: 0000000000000001
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000001001001
RBP: 0000000001001001 R8: 0074736575716552 R9: 0000000000000000
R10: ffff81003fbd0680 R11: 00000000000000c8 R12: 0000000000000008
R13: 0000000000000001 R14: 0000000000000001 R15: ffff810015c6ca00
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#4 [ffff81002d9a5ab0] tipc_node_select_next_hop at ffffffff8869d3b1
#5 [ffff81002d9a5ae0] tipc_link_send_sections_fast at ffffffff88698558
#6 [ffff81002d9a5be0] tipc_forward2port at ffffffff8869eb1d
#7 [ffff81002d9a5c10] tipc_send2port at ffffffff8869eb79
#8 [ffff81002d9a5c30] send_msg at ffffffff886a1d0b
#9 [ffff81002d9a5cb0] sock_sendmsg at ffffffff80055261
RIP: 0000003cbd8d49a3 RSP: 00007fffc84e0be8 RFLAGS: 00010206
RAX: 000000000000002c RBX: ffffffff8005d116 RCX: 0000000000000000
RDX: 0000000000000008 RSI: 00007fffc84e0c00 RDI: 0000000000000003
RBP: 0000000000000000 R8: 00007fffc84e0c10 R9: 0000000000000010
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fffc84e0d10 R14: 0000000000000000 R15: 00007fffc84e0c30
ORIG_RAX: 000000000000002c CS: 0033 SS: 002b
What happens is that, when the tipc module in inserted it enters a standalone
node mode in which communication to its own address is allowed <0.0.0> but not
to other addresses, since the appropriate data structures have not been
allocated yet (specifically the tipc_net pointer). There is nothing stopping a
client from trying to send such a message however, and if that happens, we
attempt to dereference tipc_net.zones while the pointer is still NULL, and
explode. The fix is to add a check at the start of the send_msg path to ensure
that we've allocated the tipc_net pointers and entered networked mode prior to
allowing a send to any destination other than our loopback address.
This patch has received minimal testing, but fixes the issue. Through reviews
are appreciated.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Per Liden <per.liden@ericsson.com>
CC: Jon Maloy <jon.maloy@ericsson.com>
CC: Allan Stephens <allan.stephens@windriver.com>
CC: David S. Miller <davem@davemloft.net>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: tipc-discussion@lists.sourceforge.net
net.c | 2 +-
socket.c | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 7906608..512b33c 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -278,7 +278,6 @@ int tipc_net_start(u32 addr)
tipc_cfg_stop();
tipc_own_addr = addr;
- tipc_mode = TIPC_NET_MODE;
tipc_named_reinit();
tipc_port_reinit();
@@ -289,6 +288,7 @@ int tipc_net_start(u32 addr)
return res;
}
+ tipc_mode = TIPC_NET_MODE;
tipc_k_signal((Handler)tipc_subscr_start, 0);
tipc_k_signal((Handler)tipc_cfg_init, 0);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 1ea64f0..45229fd 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -526,6 +526,10 @@ static int send_msg(struct kiocb *iocb, struct socket *sock,
if (iocb)
lock_sock(sk);
+ if ((tipc_mode != TIPC_NET_MODE) &&
+ (dest->addr.name.domain != tipc_own_addr))
+ return -EHOSTUNREACH;
+
needs_conn = (sock->state != SS_READY);
if (unlikely(needs_conn)) {
if (sock->state == SS_LISTENING) {
^ permalink raw reply related
* Re: [PATCH v2 1/3] e1000e: add registers etc. printout code just before resetting adapters
From: Laurent Chavey @ 2010-02-19 19:19 UTC (permalink / raw)
To: Taku Izumi
Cc: netdev, Bruce Allan, David S. Miller, Jesse Brandeburg,
John Ronciak, Kirsher, Jeffrey T, PJ Waskiewicz, Koki Sanagi,
Kenji Kaneshige
In-Reply-To: <4B593C33.5020208@jp.fujitsu.com>
- are you sure you want KERN_ERR rather than KERN_DEBUG ?
2010/1/21 Taku Izumi <izumi.taku@jp.fujitsu.com>:
>
> This patch adds registers (,tx/rx rings' status and so on) printout
> code just before resetting adapters. This will be helpful for detecting
> the root cause of adapters reset.
>
> The default output is netdevice status (transstart, last_rx), registers,
> and tx/rx rings' simple information. TX/RX descriptors information and
> buffer_info can be output by changing the dump_flag module option, but,
> of course, the amount of output becomes quite large.
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
> ---
> drivers/net/e1000e/hw.h | 27 +++
> drivers/net/e1000e/netdev.c | 341 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 368 insertions(+)
> Index: net-next-2.6/drivers/net/e1000e/netdev.c
> ===================================================================
> --- net-next-2.6.orig/drivers/net/e1000e/netdev.c
> +++ net-next-2.6/drivers/net/e1000e/netdev.c
> @@ -52,6 +52,14 @@
> char e1000e_driver_name[] = "e1000e";
> const char e1000e_driver_version[] = DRV_VERSION;
>
> +static unsigned int dump_flag = 1;
> +module_param(dump_flag, uint, 0644);
> +MODULE_PARM_DESC(dump_flag, "Dump Flag");
> +#define E1000_DUMP_REGS (1 << 0)
> +#define E1000_DUMP_TX_RINGS (1 << 1)
> +#define E1000_DUMP_RX_RINGS (1 << 2)
> +#define E1000_DUMP_BUFFERS (1 << 3)
> +
> static const struct e1000_info *e1000_info_tbl[] = {
> [board_82571] = &e1000_82571_info,
> [board_82572] = &e1000_82572_info,
> @@ -65,6 +73,338 @@ static const struct e1000_info *e1000_in
> [board_pchlan] = &e1000_pch_info,
> };
>
> +/*
> + * e1000_regdump - register printout routine
> + */
> +static void e1000_regdump(struct e1000_hw *hw, u32 reg,
> + char *name, u32 (*get_regofs)(int))
> +{
> + printk(KERN_ERR "%-15s %08x\n", name, __er32(hw, reg));
> +}
> +
> +/*
> + * e1000_regdump_n - register printout routine
> + */
> +static void e1000_regdump_n(struct e1000_hw *hw, u32 reg,
> + char *name, u32 (*get_regofs)(int))
> +{
> + int n = 0;
> + char rname[16];
> +
> + if (!get_regofs)
> + return;
> +
> + snprintf(rname, 16, "%s%s", name, "[0-1]");
> + printk(KERN_ERR "%-15s ", rname);
> + for (n = 0; n < 2; n++)
> + printk("%08x ", __er32(hw, get_regofs(n)));
> + printk("\n");
> +}
> +
> +static const struct e1000_reg_info e1000_reg_info_tbl[] = {
> +
> + /* General Registers */
> + {E1000_CTRL, "CTRL", NULL, e1000_regdump},
> + {E1000_STATUS, "STATUS", NULL, e1000_regdump},
> + {E1000_CTRL_EXT, "CTRL_EXT", NULL, e1000_regdump},
> +
> + /* Interrupt Registers */
> + {E1000_ICR, "ICR", NULL, e1000_regdump},
> +
> + /* RX Registers */
> + {E1000_RCTL, "RCTL", NULL, e1000_regdump},
> + {E1000_RDLEN, "RDLEN", NULL, e1000_regdump},
> + {E1000_RDH, "RDH", NULL, e1000_regdump},
> + {E1000_RDT, "RDT", NULL, e1000_regdump},
> + {E1000_RDTR, "RDTR", NULL, e1000_regdump},
> + {E1000_RXDCTL(0), "RXDCTL", get_regofs_RXDCTL, e1000_regdump_n},
> + {E1000_ERT, "ERT", NULL, e1000_regdump},
> + {E1000_RDBAL, "RDBAL", NULL, e1000_regdump},
> + {E1000_RDBAH, "RDBAH", NULL, e1000_regdump},
> + {E1000_RDFH, "RDFH", NULL, e1000_regdump},
> + {E1000_RDFT, "RDFT", NULL, e1000_regdump},
> + {E1000_RDFHS, "RDFHS", NULL, e1000_regdump},
> + {E1000_RDFTS, "RDFTS", NULL, e1000_regdump},
> + {E1000_RDFPC, "RDFPC", NULL, e1000_regdump},
> +
> + /* TX Registers */
> + {E1000_TCTL, "TCTL", NULL, e1000_regdump},
> + {E1000_TDBAL, "TDBAL", NULL, e1000_regdump},
> + {E1000_TDBAH, "TDBAH", NULL, e1000_regdump},
> + {E1000_TDLEN, "TDLEN", NULL, e1000_regdump},
> + {E1000_TDH, "TDH", NULL, e1000_regdump},
> + {E1000_TDT, "TDT", NULL, e1000_regdump},
> + {E1000_TIDV, "TIDV", NULL, e1000_regdump},
> + {E1000_TXDCTL(0), "TXDCTL", get_regofs_TXDCTL, e1000_regdump_n},
> + {E1000_TADV, "TADV", NULL, e1000_regdump},
> + {E1000_TARC(0), "TARC", get_regofs_TARC, e1000_regdump_n},
> + {E1000_TDFH, "TDFH", NULL, e1000_regdump},
> + {E1000_TDFT, "TDFT", NULL, e1000_regdump},
> + {E1000_TDFHS, "TDFHS", NULL, e1000_regdump},
> + {E1000_TDFTS, "TDFTS", NULL, e1000_regdump},
> + {E1000_TDFPC, "TDFPC", NULL, e1000_regdump},
> +
> + /* List Terminator */
> + {}
> +};
> +
> +/*
> + * e1000e_dump - Print registers, tx-ring and rx-ring
> + */
> +static void e1000e_dump(struct e1000_adapter *adapter)
> +{
> + struct net_device *netdev = adapter->netdev;
> + struct e1000_hw *hw = &adapter->hw;
> + struct e1000_reg_info *reginfo;
> + struct e1000_ring *tx_ring = adapter->tx_ring;
> + struct e1000_tx_desc *tx_desc;
> + struct my_u0 { u64 a; u64 b; } *u0;
> + struct e1000_buffer *buffer_info;
> + struct e1000_ring *rx_ring = adapter->rx_ring;
> + union e1000_rx_desc_packet_split *rx_desc_ps;
> + struct e1000_rx_desc *rx_desc;
> + struct my_u1 { u64 a; u64 b; u64 c; u64 d; } *u1;
> + u32 staterr;
> + int i = 0;
> +
> + /* Print netdevice Info */
> + if (netdev) {
> + dev_err(&adapter->pdev->dev, "Net device Info\n");
> + printk(KERN_ERR "Device Name state "
> + "trans_start last_rx\n");
> + printk(KERN_ERR "%-15s %016lX %016lX %016lX\n",
> + netdev->name,
> + netdev->state,
> + netdev->trans_start,
> + netdev->last_rx);
> + }
> +
> + /* Print Registers */
> + if ((dump_flag & E1000_DUMP_REGS) == 0)
> + goto tx_ring_summary;
> +
> + dev_err(&adapter->pdev->dev, "Register Dump\n");
> + printk(KERN_ERR " Register Name Value\n");
> + for (reginfo = (struct e1000_reg_info *)e1000_reg_info_tbl;
> + reginfo->name; reginfo++) {
> + reginfo->dump_reg(hw, reginfo->reg, reginfo->name,
> + reginfo->get_regofs);
> + }
> +
> + /* Print TX Ring Summary */
> +tx_ring_summary:
> + if (!netdev || !netif_running(netdev))
> + goto exit;
> +
> + dev_err(&adapter->pdev->dev, "TX Rings Summary\n");
> + printk(KERN_ERR "Queue [NTU] [NTC] [bi(ntc)->dma ]"
> + " leng ntw timestamp\n");
> + buffer_info = &tx_ring->buffer_info[tx_ring->next_to_clean];
> + printk(KERN_ERR " %5d %5X %5X %016llX %04X %3X %016llX\n",
> + 0, tx_ring->next_to_use, tx_ring->next_to_clean,
> + (u64)buffer_info->dma,
> + buffer_info->length,
> + buffer_info->next_to_watch,
> + (u64)buffer_info->time_stamp);
> +
> + /* Print TX Rings */
> + if ((dump_flag & E1000_DUMP_TX_RINGS) == 0)
> + goto rx_ring_summary;
> +
> + dev_err(&adapter->pdev->dev, "TX Rings Dump\n");
> +
> + /* Transmit Descriptor Formats - DEXT[29] is 0 (Legacy) or 1 (Extended)
> + *
> + * Legacy Transmit Descriptor
> + * +--------------------------------------------------------------+
> + * 0 | Buffer Address [63:0] (Reserved on Write Back) |
> + * +--------------------------------------------------------------+
> + * 8 | Special | CSS | Status | CMD | CSO | Length |
> + * +--------------------------------------------------------------+
> + * 63 48 47 36 35 32 31 24 23 16 15 0
> + *
> + * Extended Context Descriptor (DTYP=0x0) for TSO or checksum offload
> + * 63 48 47 40 39 32 31 16 15 8 7 0
> + * +----------------------------------------------------------------+
> + * 0 | TUCSE | TUCS0 | TUCSS | IPCSE | IPCS0 | IPCSS |
> + * +----------------------------------------------------------------+
> + * 8 | MSS | HDRLEN | RSV | STA | TUCMD | DTYP | PAYLEN |
> + * +----------------------------------------------------------------+
> + * 63 48 47 40 39 36 35 32 31 24 23 20 19 0
> + *
> + * Extended Data Descriptor (DTYP=0x1)
> + * +----------------------------------------------------------------+
> + * 0 | Buffer Address [63:0] |
> + * +----------------------------------------------------------------+
> + * 8 | VLAN tag | POPTS | Rsvd | Status | Command | DTYP | DTALEN |
> + * +----------------------------------------------------------------+
> + * 63 48 47 40 39 36 35 32 31 24 23 20 19 0
> + */
> + printk(KERN_ERR "Tl[desc] [address 63:0 ] [SpeCssSCmCsLen]"
> + " [bi->dma ] leng ntw timestamp bi->skb "
> + "<-- Legacy format\n");
> + printk(KERN_ERR "Tc[desc] [Ce CoCsIpceCoS] [MssHlRSCm0Plen]"
> + " [bi->dma ] leng ntw timestamp bi->skb "
> + "<-- Ext Context format\n");
> + printk(KERN_ERR "Td[desc] [address 63:0 ] [VlaPoRSCm1Dlen]"
> + " [bi->dma ] leng ntw timestamp bi->skb "
> + "<-- Ext Data format\n");
> + for (i = 0; tx_ring->desc && (i < tx_ring->count); i++) {
> + tx_desc = E1000_TX_DESC(*tx_ring, i);
> + buffer_info = &tx_ring->buffer_info[i];
> + u0 = (struct my_u0 *)tx_desc;
> + printk(KERN_ERR "T%c[0x%03X] %016llX %016llX %016llX "
> + "%04X %3X %016llX %p",
> + (!(le64_to_cpu(u0->b) & (1<<29)) ? 'l' :
> + ((le64_to_cpu(u0->b) & (1<<20)) ? 'd' : 'c')), i,
> + le64_to_cpu(u0->a), le64_to_cpu(u0->b),
> + (u64)buffer_info->dma, buffer_info->length,
> + buffer_info->next_to_watch, (u64)buffer_info->time_stamp,
> + buffer_info->skb);
> + if (i == tx_ring->next_to_use && i == tx_ring->next_to_clean)
> + printk(" NTC/U\n");
> + else if (i == tx_ring->next_to_use)
> + printk(" NTU\n");
> + else if (i == tx_ring->next_to_clean)
> + printk(" NTC\n");
> + else
> + printk("\n");
> +
> + if ((dump_flag & E1000_DUMP_BUFFERS) && buffer_info->dma != 0)
> + print_hex_dump(KERN_ERR, "", DUMP_PREFIX_ADDRESS,
> + 16, 1, phys_to_virt(buffer_info->dma),
> + buffer_info->length, true);
> + }
> +
> + /* Print RX Rings Summary */
> +rx_ring_summary:
> + dev_err(&adapter->pdev->dev, "RX Rings Summary\n");
> + printk(KERN_ERR "Queue [NTU] [NTC]\n");
> + printk(KERN_ERR " %5d %5X %5X\n", 0,
> + rx_ring->next_to_use, rx_ring->next_to_clean);
> +
> + /* Print RX Rings */
> + if ((dump_flag & E1000_DUMP_RX_RINGS) == 0)
> + goto exit;
> +
> + dev_err(&adapter->pdev->dev, "RX Rings Dump\n");
> + switch (adapter->rx_ps_pages) {
> + case 1:
> + case 2:
> + case 3:
> + /* [Extended] Packet Split Receive Descriptor Format
> + *
> + * +-----------------------------------------------------+
> + * 0 | Buffer Address 0 [63:0] |
> + * +-----------------------------------------------------+
> + * 8 | Buffer Address 1 [63:0] |
> + * +-----------------------------------------------------+
> + * 16 | Buffer Address 2 [63:0] |
> + * +-----------------------------------------------------+
> + * 24 | Buffer Address 3 [63:0] |
> + * +-----------------------------------------------------+
> + */
> + printk(KERN_ERR "R [desc] [buffer 0 63:0 ] "
> + "[buffer 1 63:0 ] "
> + "[buffer 2 63:0 ] [buffer 3 63:0 ] [bi->dma ] "
> + "[bi->skb] <-- Ext Pkt Split format\n");
> + /* [Extended] Receive Descriptor (Write-Back) Format
> + *
> + * 63 48 47 32 31 13 12 8 7 4 3 0
> + * +------------------------------------------------------+
> + * 0 | Packet | IP | Rsvd | MRQ | Rsvd | MRQ RSS |
> + * | Checksum | Ident | | Queue | | Type |
> + * +------------------------------------------------------+
> + * 8 | VLAN Tag | Length | Extended Error | Extended Status |
> + * +------------------------------------------------------+
> + * 63 48 47 32 31 20 19 0
> + */
> + printk(KERN_ERR "RWB[desc] [ck ipid mrqhsh] "
> + "[vl l0 ee es] "
> + "[ l3 l2 l1 hs] [reserved ] ---------------- "
> + "[bi->skb] <-- Ext Rx Write-Back format\n");
> + for (i = 0; i < rx_ring->count; i++) {
> + buffer_info = &rx_ring->buffer_info[i];
> + rx_desc_ps = E1000_RX_DESC_PS(*rx_ring, i);
> + u1 = (struct my_u1 *)rx_desc_ps;
> + staterr =
> + le32_to_cpu(rx_desc_ps->wb.middle.status_error);
> + if (staterr & E1000_RXD_STAT_DD) {
> + /* Descriptor Done */
> + printk(KERN_ERR "RWB[0x%03X] %016llX "
> + "%016llX %016llX %016llX "
> + "---------------- %p", i,
> + le64_to_cpu(u1->a),
> + le64_to_cpu(u1->b),
> + le64_to_cpu(u1->c),
> + le64_to_cpu(u1->d),
> + buffer_info->skb);
> + } else {
> + printk(KERN_ERR "R [0x%03X] %016llX "
> + "%016llX %016llX %016llX %016llX %p", i,
> + le64_to_cpu(u1->a),
> + le64_to_cpu(u1->b),
> + le64_to_cpu(u1->c),
> + le64_to_cpu(u1->d),
> + (u64)buffer_info->dma,
> + buffer_info->skb);
> +
> + if (dump_flag & E1000_DUMP_BUFFERS)
> + print_hex_dump(KERN_ERR, "",
> + DUMP_PREFIX_ADDRESS, 16, 1,
> + phys_to_virt(buffer_info->dma),
> + adapter->rx_ps_bsize0, true);
> + }
> +
> + if (i == rx_ring->next_to_use)
> + printk(" NTU\n");
> + else if (i == rx_ring->next_to_clean)
> + printk(" NTC\n");
> + else
> + printk("\n");
> + }
> + break;
> + default:
> + case 0:
> + /* Legacy Receive Descriptor Format
> + *
> + * +-----------------------------------------------------+
> + * | Buffer Address [63:0] |
> + * +-----------------------------------------------------+
> + * | VLAN Tag | Errors | Status 0 | Packet csum | Length |
> + * +-----------------------------------------------------+
> + * 63 48 47 40 39 32 31 16 15 0
> + */
> + printk(KERN_ERR "Rl[desc] [address 63:0 ] "
> + "[vl er S cks ln] [bi->dma ] [bi->skb] "
> + "<-- Legacy format\n");
> + for (i = 0; rx_ring->desc && (i < rx_ring->count); i++) {
> + rx_desc = E1000_RX_DESC(*rx_ring, i);
> + buffer_info = &rx_ring->buffer_info[i];
> + u0 = (struct my_u0 *)rx_desc;
> + printk(KERN_ERR "Rl[0x%03X] %016llX %016llX "
> + "%016llX %p",
> + i, le64_to_cpu(u0->a), le64_to_cpu(u0->b),
> + (u64)buffer_info->dma, buffer_info->skb);
> + if (i == rx_ring->next_to_use)
> + printk(" NTU\n");
> + else if (i == rx_ring->next_to_clean)
> + printk(" NTC\n");
> + else
> + printk("\n");
> +
> + if (dump_flag & E1000_DUMP_BUFFERS)
> + print_hex_dump(KERN_ERR, "",
> + DUMP_PREFIX_ADDRESS,
> + 16, 1, phys_to_virt(buffer_info->dma),
> + adapter->rx_buffer_len, true);
> + }
> + }
> +
> +exit:
> + return;
> +}
> +
> /**
> * e1000_desc_unused - calculate if we have unused descriptors
> **/
> @@ -4247,6 +4587,7 @@ static void e1000_reset_task(struct work
> struct e1000_adapter *adapter;
> adapter = container_of(work, struct e1000_adapter, reset_task);
>
> + e1000e_dump(adapter);
> e1000e_reinit_locked(adapter);
> }
>
> Index: net-next-2.6/drivers/net/e1000e/hw.h
> ===================================================================
> --- net-next-2.6.orig/drivers/net/e1000e/hw.h
> +++ net-next-2.6/drivers/net/e1000e/hw.h
> @@ -92,6 +92,11 @@ enum e1e_registers {
> E1000_FCRTL = 0x02160, /* Flow Control Receive Threshold Low - RW */
> E1000_FCRTH = 0x02168, /* Flow Control Receive Threshold High - RW */
> E1000_PSRCTL = 0x02170, /* Packet Split Receive Control - RW */
> + E1000_RDFH = 0x02410, /* Rx Data FIFO Head - RW */
> + E1000_RDFT = 0x02418, /* Rx Data FIFO Tail - RW */
> + E1000_RDFHS = 0x02420, /* Rx Data FIFO Head Saved - RW */
> + E1000_RDFTS = 0x02428, /* Rx Data FIFO Tail Saved - RW */
> + E1000_RDFPC = 0x02430, /* Rx Data FIFO Packet Count - RW */
> E1000_RDBAL = 0x02800, /* Rx Descriptor Base Address Low - RW */
> E1000_RDBAH = 0x02804, /* Rx Descriptor Base Address High - RW */
> E1000_RDLEN = 0x02808, /* Rx Descriptor Length - RW */
> @@ -101,6 +106,11 @@ enum e1e_registers {
> E1000_RXDCTL_BASE = 0x02828, /* Rx Descriptor Control - RW */
> #define E1000_RXDCTL(_n) (E1000_RXDCTL_BASE + (_n << 8))
> E1000_RADV = 0x0282C, /* RX Interrupt Absolute Delay Timer - RW */
> + E1000_TDFH = 0x03410, /* Tx Data FIFO Head - RW */
> + E1000_TDFT = 0x03418, /* Tx Data FIFO Tail - RW */
> + E1000_TDFHS = 0x03420, /* Tx Data FIFO Head Saved - RW */
> + E1000_TDFTS = 0x03428, /* Tx Data FIFO Tail Saved - RW */
> + E1000_TDFPC = 0x03430, /* Tx Data FIFO Packet Count - RW */
>
> /* Convenience macros
> *
> @@ -219,6 +229,23 @@ enum e1e_registers {
> E1000_HICR = 0x08F00, /* Host Interface Control */
> };
>
> +struct e1000_reg_info {
> + u32 reg;
> + char *name;
> + u32 (*get_regofs)(int);
> + void (*dump_reg)(struct e1000_hw *, u32, char *, u32 (*)(int));
> +};
> +
> +#define E1000_GET_REGOFS_FUNC(name) \
> +static inline u32 get_regofs_##name(int n) \
> +{ \
> + return E1000_##name(n); \
> +} \
> +
> +E1000_GET_REGOFS_FUNC(RXDCTL)
> +E1000_GET_REGOFS_FUNC(TXDCTL)
> +E1000_GET_REGOFS_FUNC(TARC)
> +
> #define E1000_MAX_PHY_ADDR 4
>
> /* IGP01E1000 Specific Registers */
>
>
^ permalink raw reply
* Re: [RFC 1/2] sctp: convert hash list to RCU
From: Paul E. McKenney @ 2010-02-19 19:07 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Vlad Yasevich, Sridhar Samudrala, David S. Miller, netdev,
linux-sctp
In-Reply-To: <20100219081856.79ba7cf8@nehalam>
On Fri, Feb 19, 2010 at 08:18:56AM -0800, Stephen Hemminger wrote:
> On Fri, 19 Feb 2010 07:58:25 -0800
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>
> > On Thu, Feb 18, 2010 at 09:55:21PM -0800, Stephen Hemminger wrote:
> > > This patch converts existing SCTP hash list to using RCU
> > > rather than reader/writer lock. Also, get rid of no longer used
> > > locking wrappers.
> > >
> > > In future, the SCTP hash locking should be broken out from the
> > > hash structure because of the wasted space for the hash locks
> > > and associated holes. A single lock per hashlist is sufficient
> > > now that RCU is used.
> > >
> > > Compile tested only. I can't think of an SCTP stress application.
> > >
> > > P.s: Some janitor ought to go through and remove the locking
> > > macros here.
> >
> > One question below about what looks to be mixing of RCU and RCU-bh
> > read-side critical sections while waiting only for RCU grace periods.
> > Unless I am missing something, this can result in memory corruption.
>
> Thanks, I copied the original locking which was broken there as well.
I know that feeling! :-)
Thanx, Paul
^ permalink raw reply
* [net-next 2.6 PATCH] net/pcmcia: convert to use netdev_for_each_mc_addr
From: Jiri Pirko @ 2010-02-19 18:48 UTC (permalink / raw)
To: netdev; +Cc: davem
removed fill_multicast_tbl function in smc91c92_cs and do the work inline
rewritten set_addresses function in xirc2ps_cs. This was kinda headache.
Simulated the original and new functions and they bahave the same.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/pcmcia/axnet_cs.c | 7 +--
drivers/net/pcmcia/fmvj18x_cs.c | 6 +--
drivers/net/pcmcia/nmclan_cs.c | 6 +--
drivers/net/pcmcia/smc91c92_cs.c | 33 +++++----------
drivers/net/pcmcia/xirc2ps_cs.c | 84 ++++++++++++++++++++++---------------
5 files changed, 67 insertions(+), 69 deletions(-)
diff --git a/drivers/net/pcmcia/axnet_cs.c b/drivers/net/pcmcia/axnet_cs.c
index 2ee57bd..d90e0fc 100644
--- a/drivers/net/pcmcia/axnet_cs.c
+++ b/drivers/net/pcmcia/axnet_cs.c
@@ -1625,8 +1625,7 @@ static inline void make_mc_bits(u8 *bits, struct net_device *dev)
struct dev_mc_list *dmi;
u32 crc;
- for (dmi=dev->mc_list; dmi; dmi=dmi->next) {
-
+ netdev_for_each_mc_addr(dmi, dev) {
crc = ether_crc(ETH_ALEN, dmi->dmi_addr);
/*
* The 8390 uses the 6 most significant bits of the
@@ -1652,7 +1651,7 @@ static void do_set_multicast_list(struct net_device *dev)
if (!(dev->flags&(IFF_PROMISC|IFF_ALLMULTI))) {
memset(ei_local->mcfilter, 0, 8);
- if (dev->mc_list)
+ if (!netdev_mc_empty(dev))
make_mc_bits(ei_local->mcfilter, dev);
} else {
/* set to accept-all */
@@ -1668,7 +1667,7 @@ static void do_set_multicast_list(struct net_device *dev)
if(dev->flags&IFF_PROMISC)
outb_p(E8390_RXCONFIG | 0x58, e8390_base + EN0_RXCR);
- else if(dev->flags&IFF_ALLMULTI || dev->mc_list)
+ else if (dev->flags & IFF_ALLMULTI || !netdev_mc_empty(dev))
outb_p(E8390_RXCONFIG | 0x48, e8390_base + EN0_RXCR);
else
outb_p(E8390_RXCONFIG | 0x40, e8390_base + EN0_RXCR);
diff --git a/drivers/net/pcmcia/fmvj18x_cs.c b/drivers/net/pcmcia/fmvj18x_cs.c
index 3d573ed..b9dc80b 100644
--- a/drivers/net/pcmcia/fmvj18x_cs.c
+++ b/drivers/net/pcmcia/fmvj18x_cs.c
@@ -1199,10 +1199,8 @@ static void set_rx_mode(struct net_device *dev)
struct dev_mc_list *mclist;
memset(mc_filter, 0, sizeof(mc_filter));
- for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
- i++, mclist = mclist->next) {
- unsigned int bit =
- ether_crc_le(ETH_ALEN, mclist->dmi_addr) >> 26;
+ netdev_for_each_mc_addr(mclist, dev) {
+ unsigned int bit = ether_crc_le(ETH_ALEN, mclist->dmi_addr) >> 26;
mc_filter[bit >> 3] |= (1 << (bit & 7));
}
outb(2, ioaddr + RX_MODE); /* Use normal mode. */
diff --git a/drivers/net/pcmcia/nmclan_cs.c b/drivers/net/pcmcia/nmclan_cs.c
index c42a31a..c717b14 100644
--- a/drivers/net/pcmcia/nmclan_cs.c
+++ b/drivers/net/pcmcia/nmclan_cs.c
@@ -1475,8 +1475,7 @@ static void set_multicast_list(struct net_device *dev)
{
mace_private *lp = netdev_priv(dev);
int adr[ETHER_ADDR_LEN] = {0}; /* Ethernet address */
- int i;
- struct dev_mc_list *dmi = dev->mc_list;
+ struct dev_mc_list *dmi;
#ifdef PCMCIA_DEBUG
{
@@ -1496,9 +1495,8 @@ static void set_multicast_list(struct net_device *dev)
if (num_addrs > 0) {
/* Calculate multicast logical address filter */
memset(lp->multicast_ladrf, 0, MACE_LADRF_LEN);
- for (i = 0; i < netdev_mc_count(dev); i++) {
+ netdev_for_each_mc_addr(dmi, dev) {
memcpy(adr, dmi->dmi_addr, ETHER_ADDR_LEN);
- dmi = dmi->next;
BuildLAF(lp->multicast_ladrf, adr);
}
}
diff --git a/drivers/net/pcmcia/smc91c92_cs.c b/drivers/net/pcmcia/smc91c92_cs.c
index d2e86b8..d29c22a 100644
--- a/drivers/net/pcmcia/smc91c92_cs.c
+++ b/drivers/net/pcmcia/smc91c92_cs.c
@@ -1595,27 +1595,6 @@ static void smc_rx(struct net_device *dev)
/*======================================================================
- Calculate values for the hardware multicast filter hash table.
-
-======================================================================*/
-
-static void fill_multicast_tbl(int count, struct dev_mc_list *addrs,
- u_char *multicast_table)
-{
- struct dev_mc_list *mc_addr;
-
- for (mc_addr = addrs; mc_addr && count-- > 0; mc_addr = mc_addr->next) {
- u_int position = ether_crc(6, mc_addr->dmi_addr);
-#ifndef final_version /* Verify multicast address. */
- if ((mc_addr->dmi_addr[0] & 1) == 0)
- continue;
-#endif
- multicast_table[position >> 29] |= 1 << ((position >> 26) & 7);
- }
-}
-
-/*======================================================================
-
Set the receive mode.
This routine is used by both the protocol level to notify us of
@@ -1639,8 +1618,16 @@ static void set_rx_mode(struct net_device *dev)
rx_cfg_setting = RxStripCRC | RxEnable | RxAllMulti;
else {
if (!netdev_mc_empty(dev)) {
- fill_multicast_tbl(netdev_mc_count(dev), dev->mc_list,
- (u_char *)multicast_table);
+ struct dev_mc_list *mc_addr;
+
+ netdev_for_each_mc_addr(mc_addr, dev) {
+ u_int position = ether_crc(6, mc_addr->dmi_addr);
+#ifndef final_version /* Verify multicast address. */
+ if ((mc_addr->dmi_addr[0] & 1) == 0)
+ continue;
+#endif
+ multicast_table[position >> 29] |= 1 << ((position >> 26) & 7);
+ }
}
rx_cfg_setting = RxStripCRC | RxEnable;
}
diff --git a/drivers/net/pcmcia/xirc2ps_cs.c b/drivers/net/pcmcia/xirc2ps_cs.c
index 4ace18a..4d1802e 100644
--- a/drivers/net/pcmcia/xirc2ps_cs.c
+++ b/drivers/net/pcmcia/xirc2ps_cs.c
@@ -1364,47 +1364,63 @@ do_start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
+struct set_address_info {
+ int reg_nr;
+ int page_nr;
+ int mohawk;
+ unsigned int ioaddr;
+};
+
+static void set_address(struct set_address_info *sa_info, char *addr)
+{
+ unsigned int ioaddr = sa_info->ioaddr;
+ int i;
+
+ for (i = 0; i < 6; i++) {
+ if (sa_info->reg_nr > 15) {
+ sa_info->reg_nr = 8;
+ sa_info->page_nr++;
+ SelectPage(sa_info->page_nr);
+ }
+ if (sa_info->mohawk)
+ PutByte(sa_info->reg_nr++, addr[5 - i]);
+ else
+ PutByte(sa_info->reg_nr++, addr[i]);
+ }
+}
+
/****************
* Set all addresses: This first one is the individual address,
* the next 9 addresses are taken from the multicast list and
* the rest is filled with the individual address.
*/
-static void
-set_addresses(struct net_device *dev)
+static void set_addresses(struct net_device *dev)
{
- unsigned int ioaddr = dev->base_addr;
- local_info_t *lp = netdev_priv(dev);
- struct dev_mc_list *dmi = dev->mc_list;
- unsigned char *addr;
- int i,j,k,n;
-
- SelectPage(k=0x50);
- for (i=0,j=8,n=0; ; i++, j++) {
- if (i > 5) {
- if (++n > 9)
- break;
- i = 0;
- if (n > 1 && n <= netdev_mc_count(dev) && dmi) {
- dmi = dmi->next;
- }
- }
- if (j > 15) {
- j = 8;
- k++;
- SelectPage(k);
- }
-
- if (n && n <= netdev_mc_count(dev) && dmi)
- addr = dmi->dmi_addr;
- else
- addr = dev->dev_addr;
+ unsigned int ioaddr = dev->base_addr;
+ local_info_t *lp = netdev_priv(dev);
+ struct dev_mc_list *dmi;
+ struct set_address_info sa_info;
+ int i;
- if (lp->mohawk)
- PutByte(j, addr[5-i]);
- else
- PutByte(j, addr[i]);
- }
- SelectPage(0);
+ /*
+ * Setup the info structure so that by first set_address call it will do
+ * SelectPage with the right page number. Hence these ones here.
+ */
+ sa_info.reg_nr = 15 + 1;
+ sa_info.page_nr = 0x50 - 1;
+ sa_info.mohawk = lp->mohawk;
+ sa_info.ioaddr = ioaddr;
+
+ set_address(&sa_info, dev->dev_addr);
+ i = 0;
+ netdev_for_each_mc_addr(dmi, dev) {
+ if (i++ == 9)
+ break;
+ set_address(&sa_info, dmi->dmi_addr);
+ }
+ while (i++ < 9)
+ set_address(&sa_info, dev->dev_addr);
+ SelectPage(0);
}
/****************
--
1.6.6
^ permalink raw reply related
* Re: NAT regression in next tree
From: Stephen Hemminger @ 2010-02-19 18:11 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Eric Dumazet, David Miller, netdev, netfilter-devel
In-Reply-To: <4B7E3D65.2030203@trash.net>
On Fri, 19 Feb 2010 08:27:33 +0100
Patrick McHardy <kaber@trash.net> wrote:
> Eric Dumazet wrote:
> > Le vendredi 19 février 2010 à 08:06 +0100, Patrick McHardy a écrit :
> >> netfilter: restore POST_ROUTING hook in NF_HOOK_COND
> >>
> >> Commit 2249065 ("netfilter: get rid of the grossness in netfilter.h")
> >> inverted the logic for conditional hook invocation, breaking the
> >> POST_ROUTING hook invoked by ip_output().
> >>
> >> Correct the logic and remove an unnecessary initialization.
> >>
> >> Reported-by: Stephen Hemminger <shemminger@vyatta.com>
> >> Signed-off-by: Patrick McHardy <kaber@trash.net>
> >>
> >> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> >> index 7007945..89341c3 100644
> >> --- a/include/linux/netfilter.h
> >> +++ b/include/linux/netfilter.h
> >> @@ -212,8 +212,9 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
> >> struct net_device *in, struct net_device *out,
> >> int (*okfn)(struct sk_buff *), bool cond)
> >> {
> >> - int ret = 1;
> >> - if (cond ||
> >> + int ret;
> >> +
> >> + if (!cond ||
> >> (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
> >> ret = okfn(skb);
> >> return ret;
> >
> > I dont quite get it
> >
> > Original code was :
> >
> >
> > #define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond) \
> > ({int __ret; \
> > if ((cond) || (__ret = nf_hook_thresh(pf, hook, (skb), indev, outdev, okfn, INT_MIN)) == 1)\
> > __ret = (okfn)(skb); \
> > __ret;})
> >
> >
> > There was no condition inversion.
>
> Right, I quoted the wrong patch, it was actually broken in
> 23f3733 ("netfilter: reduce NF_HOOK by one argument"), which
> moved the cond check from nf_hook_thresh() to NF_HOOK_COND().
Yes, this fixes the problem I was seeing.
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
--
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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: [RFC 4/4] bond: add new multiqueue hash policy
From: "Oleg A. Arkhangelsky" @ 2010-02-19 17:54 UTC (permalink / raw)
To: netdev
Hello Stephen,
> This hash policy just uses existing transmit hash used for
> mulitiqueue.
> Signed-off-by: Stephen Hemminger
> +/*
> + * Hash for the output device based on multiqueue transmit hash policy
> + */
> +static u16 bond_xmit_hash_policy_mq(const struct sk_buff *skb)
> +{
> + return skb_tx_hash(skb->dev, skb);
> +}
I think that it would be better to use something like this here:
return skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
Bonding is currently non-multiqueue aware and skb_tx_hash() will return zero
everytime.
--
wbr, Oleg.
^ permalink raw reply
* Re: [RFC 1/2] sctp: convert hash list to RCU
From: Vlad Yasevich @ 2010-02-19 17:29 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Sridhar Samudrala, David S. Miller, Paul E. McKenney, netdev,
linux-sctp
In-Reply-To: <20100219055628.436258223@vyatta.com>
Hi Stephen
Som comments below...
> This patch converts existing SCTP hash list to using RCU
> rather than reader/writer lock. Also, get rid of no longer used
> locking wrappers.
>
> In future, the SCTP hash locking should be broken out from the
> hash structure because of the wasted space for the hash locks
> and associated holes. A single lock per hashlist is sufficient
> now that RCU is used.
>
> Compile tested only. I can't think of an SCTP stress application.
>
> P.s: Some janitor ought to go through and remove the locking
> macros here.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
> --- a/include/net/sctp/sctp.h 2010-02-18 09:44:42.664390403 -0800
> +++ b/include/net/sctp/sctp.h 2010-02-18 09:52:48.004478538 -0800
> @@ -206,10 +206,6 @@ extern struct kmem_cache *sctp_bucket_ca
> #define sctp_local_bh_enable() local_bh_enable()
> #define sctp_spin_lock(lock) spin_lock(lock)
> #define sctp_spin_unlock(lock) spin_unlock(lock)
> -#define sctp_write_lock(lock) write_lock(lock)
> -#define sctp_write_unlock(lock) write_unlock(lock)
> -#define sctp_read_lock(lock) read_lock(lock)
> -#define sctp_read_unlock(lock) read_unlock(lock)
>
> /* sock lock wrappers. */
> #define sctp_lock_sock(sk) lock_sock(sk)
> @@ -649,6 +645,9 @@ static inline int sctp_vtag_hashfn(__u16
> #define sctp_for_each_hentry(epb, node, head) \
> hlist_for_each_entry(epb, node, head, node)
>
> +#define sctp_for_each_hentry_rcu(epb, node, head) \
> + hlist_for_each_entry_rcu(epb, node, head, node)
> +
> /* Is a socket of this style? */
> #define sctp_style(sk, style) __sctp_style((sk), (SCTP_SOCKET_##style))
> static inline int __sctp_style(const struct sock *sk, sctp_socket_type_t style)
> --- a/include/net/sctp/structs.h 2010-02-18 09:47:29.964390311 -0800
> +++ b/include/net/sctp/structs.h 2010-02-18 09:57:26.792896287 -0800
> @@ -111,7 +111,7 @@ struct sctp_bind_hashbucket {
>
> /* Used for hashing all associations. */
> struct sctp_hashbucket {
> - rwlock_t lock;
> + spinlock_t lock;
> struct hlist_head chain;
> } __attribute__((__aligned__(8)));
>
> @@ -1371,6 +1371,8 @@ struct sctp_endpoint {
> /* SCTP-AUTH: endpoint shared keys */
> struct list_head endpoint_shared_keys;
> __u16 active_key_id;
> +
> + struct rcu_head rcu;
> };
>
I'd recommend putting this into sctp_ep_common structure since that's
what actually gets hashed. That structure is contained with sctp_endpoint
as well as sctp_association.
> /* Recover the outter endpoint structure. */
> --- a/net/sctp/endpointola.c 2010-02-18 09:43:22.868887786 -0800
> +++ b/net/sctp/endpointola.c 2010-02-18 10:00:55.807210206 -0800
> @@ -247,9 +247,9 @@ void sctp_endpoint_free(struct sctp_endp
> }
>
> /* Final destructor for endpoint. */
> -static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
> +static void sctp_endpoint_destroy_rcu(struct rcu_head *rcu)
> {
> - SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
> + struct sctp_endpoint *ep = container_of(rcu, struct sctp_endpoint, rcu);
>
> /* Free up the HMAC transform. */
> crypto_free_hash(sctp_sk(ep->base.sk)->hmac);
> @@ -286,6 +286,13 @@ static void sctp_endpoint_destroy(struct
> }
> }
>
> +static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
> +{
> + SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
> + call_rcu(&ep->rcu, sctp_endpoint_destroy_rcu);
> +}
> +
> +
You will need to do the same thing for sctp_association_free(), since right now
you are using rcu to locate it (replaced the read lock), but the destruction
sequence does not wait for the rcu grace period.
> /* Hold a reference to an endpoint. */
> void sctp_endpoint_hold(struct sctp_endpoint *ep)
> {
> @@ -338,8 +345,9 @@ static struct sctp_association *__sctp_e
>
> hash = sctp_assoc_hashfn(ep->base.bind_addr.port, rport);
> head = &sctp_assoc_hashtable[hash];
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> asoc = sctp_assoc(epb);
> if (asoc->ep != ep || rport != asoc->peer.port)
> goto next;
> @@ -352,7 +360,7 @@ static struct sctp_association *__sctp_e
> next:
> asoc = NULL;
> }
> - read_unlock(&head->lock);
> + rcu_read_unlock();
> return asoc;
> }
>
> --- a/net/sctp/input.c 2010-02-18 09:51:39.104889498 -0800
> +++ b/net/sctp/input.c 2010-02-18 10:02:42.972601804 -0800
> @@ -704,9 +704,9 @@ static void __sctp_hash_endpoint(struct
> epb->hashent = sctp_ep_hashfn(epb->bind_addr.port);
> head = &sctp_ep_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - hlist_add_head(&epb->node, &head->chain);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_add_head_rcu(&epb->node, &head->chain);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Add an endpoint to the hash. Local BH-safe. */
> @@ -732,9 +732,9 @@ static void __sctp_unhash_endpoint(struc
>
> head = &sctp_ep_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - __hlist_del(&epb->node);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_del_rcu(&epb->node);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Remove endpoint from the hash. Local BH-safe. */
> @@ -756,8 +756,8 @@ static struct sctp_endpoint *__sctp_rcv_
>
> hash = sctp_ep_hashfn(ntohs(laddr->v4.sin_port));
> head = &sctp_ep_hashtable[hash];
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> + rcu_read_lock();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> ep = sctp_ep(epb);
> if (sctp_endpoint_is_match(ep, laddr))
> goto hit;
> @@ -767,7 +767,7 @@ static struct sctp_endpoint *__sctp_rcv_
>
> hit:
> sctp_endpoint_hold(ep);
> - read_unlock(&head->lock);
> + rcu_read_unlock();
> return ep;
> }
>
> @@ -784,9 +784,9 @@ static void __sctp_hash_established(stru
>
> head = &sctp_assoc_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - hlist_add_head(&epb->node, &head->chain);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_add_head_rcu(&epb->node, &head->chain);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Add an association to the hash. Local BH-safe. */
> @@ -813,9 +813,9 @@ static void __sctp_unhash_established(st
>
> head = &sctp_assoc_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - __hlist_del(&epb->node);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_del_rcu(&epb->node);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Remove association from the hash table. Local BH-safe. */
> @@ -847,22 +847,23 @@ static struct sctp_association *__sctp_l
> */
> hash = sctp_assoc_hashfn(ntohs(local->v4.sin_port), ntohs(peer->v4.sin_port));
> head = &sctp_assoc_hashtable[hash];
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> asoc = sctp_assoc(epb);
> transport = sctp_assoc_is_match(asoc, local, peer);
> if (transport)
> goto hit;
> }
>
> - read_unlock(&head->lock);
> + rcu_read_unlock();
>
> return NULL;
>
> hit:
> *pt = transport;
> sctp_association_hold(asoc);
> - read_unlock(&head->lock);
> + rcu_read_unlock();
> return asoc;
> }
>
> --- a/net/sctp/protocol.c 2010-02-18 09:42:04.556389428 -0800
> +++ b/net/sctp/protocol.c 2010-02-18 09:53:03.360663641 -0800
> @@ -1204,7 +1204,7 @@ SCTP_STATIC __init int sctp_init(void)
> goto err_ahash_alloc;
> }
> for (i = 0; i < sctp_assoc_hashsize; i++) {
> - rwlock_init(&sctp_assoc_hashtable[i].lock);
> + spin_lock_init(&sctp_assoc_hashtable[i].lock);
> INIT_HLIST_HEAD(&sctp_assoc_hashtable[i].chain);
> }
>
> @@ -1218,7 +1218,7 @@ SCTP_STATIC __init int sctp_init(void)
> goto err_ehash_alloc;
> }
> for (i = 0; i < sctp_ep_hashsize; i++) {
> - rwlock_init(&sctp_ep_hashtable[i].lock);
> + spin_lock_init(&sctp_ep_hashtable[i].lock);
> INIT_HLIST_HEAD(&sctp_ep_hashtable[i].chain);
> }
>
> --- a/net/sctp/proc.c 2010-02-18 10:03:50.428764115 -0800
> +++ b/net/sctp/proc.c 2010-02-18 10:05:09.961659526 -0800
> @@ -208,9 +208,9 @@ static int sctp_eps_seq_show(struct seq_
> return -ENOMEM;
>
> head = &sctp_ep_hashtable[hash];
> - sctp_local_bh_disable();
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock_bh();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> ep = sctp_ep(epb);
> sk = epb->sk;
> seq_printf(seq, "%8p %8p %-3d %-3d %-4d %-5d %5d %5lu ", ep, sk,
> @@ -221,8 +221,7 @@ static int sctp_eps_seq_show(struct seq_
> sctp_seq_dump_local_addrs(seq, epb);
> seq_printf(seq, "\n");
> }
> - read_unlock(&head->lock);
> - sctp_local_bh_enable();
> + rcu_read_unlock_bh();
>
> return 0;
> }
> @@ -312,9 +311,9 @@ static int sctp_assocs_seq_show(struct s
> return -ENOMEM;
>
> head = &sctp_assoc_hashtable[hash];
> - sctp_local_bh_disable();
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock_bh();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> assoc = sctp_assoc(epb);
> sk = epb->sk;
> seq_printf(seq,
> @@ -339,8 +338,7 @@ static int sctp_assocs_seq_show(struct s
> assoc->rtx_data_chunks);
> seq_printf(seq, "\n");
> }
> - read_unlock(&head->lock);
> - sctp_local_bh_enable();
> + rcu_read_unlock_bh();
>
> return 0;
> }
I think you need to do here what was done for TCP, i.e. convert this to
writers and take a spin_lock on the bucket making sure that the chain doesn't
change while we are printing it out.
-vlad
> @@ -425,9 +423,9 @@ static int sctp_remaddr_seq_show(struct
> return -ENOMEM;
>
> head = &sctp_assoc_hashtable[hash];
> - sctp_local_bh_disable();
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock_bh();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> assoc = sctp_assoc(epb);
> list_for_each_entry(tsp, &assoc->peer.transport_addr_list,
> transports) {
> @@ -475,9 +473,7 @@ static int sctp_remaddr_seq_show(struct
> seq_printf(seq, "\n");
> }
> }
> -
> - read_unlock(&head->lock);
> - sctp_local_bh_enable();
> + rcu_read_unlock_bh();
>
> return 0;
>
>
> --
^ permalink raw reply
* Mr. Ben Idriss
From: Ben Idriss @ 2010-02-19 17:16 UTC (permalink / raw)
Dear Friend
I am Mr. Ben Idriss a banker in Islamic development bank I contacted you now on a business deal of US$10.5m (ten million five hundred thousand united states dollar). to transfer to your account the depositor of the said fund died with his entire family during the Iraq war in 2006. unfortunately leaving nobody for the claim. if we make this claim, we will share it 50%/50%. there is no risk involved in this transaction If you are interested reply to this my private mail address benidriss02@gmail.com for more details.about this transaction I shall be waiting for your reply
Best regard
Mr. Ben Idriss
^ permalink raw reply
* netfilter 03/03: nf_queue: fix NF_STOLEN skb leak
From: Patrick McHardy @ 2010-02-19 17:02 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100219170206.18096.12788.sendpatchset@x2.localnet>
commit 64507fdbc29c3a622180378210ecea8659b14e40
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri Feb 19 15:28:38 2010 +0100
netfilter: nf_queue: fix NF_STOLEN skb leak
commit 3bc38712e3a6e059 (handle NF_STOP and unknown verdicts in
nf_reinject) was a partial fix to packet leaks.
If user asks NF_STOLEN status, we must free the skb as well.
Reported-by: Afi Gjermund <afigjermund@gmail.com>
Signed-off-by: Eric DUmazet <eric.dumazet@gmail.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 3a6fd77..ba095fd 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -265,7 +265,6 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
local_bh_disable();
entry->okfn(skb);
local_bh_enable();
- case NF_STOLEN:
break;
case NF_QUEUE:
if (!__nf_queue(skb, elem, entry->pf, entry->hook,
@@ -273,6 +272,7 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
verdict >> NF_VERDICT_BITS))
goto next_hook;
break;
+ case NF_STOLEN:
default:
kfree_skb(skb);
}
^ permalink raw reply related
* netfilter 02/03: ctnetlink: fix creation of conntrack with helpers
From: Patrick McHardy @ 2010-02-19 17:02 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100219170206.18096.12788.sendpatchset@x2.localnet>
commit a88e22adf5aad79b6e2ddd1bf0109c2ba8b46b0e
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri Feb 19 14:24:39 2010 +0100
netfilter: ctnetlink: fix creation of conntrack with helpers
This patch fixes a bug that triggers an assertion if you create
a conntrack entry with a helper and netfilter debugging is enabled.
Basically, we hit the assertion because the confirmation flag is
set before the conntrack extensions are added. To fix this, we
move the extension addition before the aforementioned flag is
set.
This patch also removes the possibility of setting a helper for
existing conntracks. This operation would also trigger the
assertion since we are not allowed to add new extensions for
existing conntracks. We know noone that could benefit from
this operation sanely.
Thanks to Eric Dumazet for initial posting a preliminary patch
to address this issue.
Reported-by: David Ramblewski <David.Ramblewski@atosorigin.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 8b05f36..2b2af63 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1077,9 +1077,8 @@ ctnetlink_change_helper(struct nf_conn *ct, const struct nlattr * const cda[])
/* need to zero data of old helper */
memset(&help->help, 0, sizeof(help->help));
} else {
- help = nf_ct_helper_ext_add(ct, GFP_ATOMIC);
- if (help == NULL)
- return -ENOMEM;
+ /* we cannot set a helper for an existing conntrack */
+ return -EOPNOTSUPP;
}
rcu_assign_pointer(help->helper, helper);
@@ -1263,7 +1262,6 @@ ctnetlink_create_conntrack(struct net *net, u16 zone,
ct->timeout.expires = ntohl(nla_get_be32(cda[CTA_TIMEOUT]));
ct->timeout.expires = jiffies + ct->timeout.expires * HZ;
- ct->status |= IPS_CONFIRMED;
rcu_read_lock();
if (cda[CTA_HELP]) {
@@ -1314,14 +1312,19 @@ ctnetlink_create_conntrack(struct net *net, u16 zone,
goto err2;
}
- if (cda[CTA_STATUS]) {
- err = ctnetlink_change_status(ct, cda);
+ if (cda[CTA_NAT_SRC] || cda[CTA_NAT_DST]) {
+ err = ctnetlink_change_nat(ct, cda);
if (err < 0)
goto err2;
}
- if (cda[CTA_NAT_SRC] || cda[CTA_NAT_DST]) {
- err = ctnetlink_change_nat(ct, cda);
+ nf_ct_acct_ext_add(ct, GFP_ATOMIC);
+ nf_ct_ecache_ext_add(ct, 0, 0, GFP_ATOMIC);
+ /* we must add conntrack extensions before confirmation. */
+ ct->status |= IPS_CONFIRMED;
+
+ if (cda[CTA_STATUS]) {
+ err = ctnetlink_change_status(ct, cda);
if (err < 0)
goto err2;
}
@@ -1340,9 +1343,6 @@ ctnetlink_create_conntrack(struct net *net, u16 zone,
goto err2;
}
- nf_ct_acct_ext_add(ct, GFP_ATOMIC);
- nf_ct_ecache_ext_add(ct, 0, 0, GFP_ATOMIC);
-
#if defined(CONFIG_NF_CONNTRACK_MARK)
if (cda[CTA_MARK])
ct->mark = ntohl(nla_get_be32(cda[CTA_MARK]));
^ permalink raw reply related
* netfilter 00/03: netfilter fixes
From: Patrick McHardy @ 2010-02-19 17:02 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
Hi Dave,
following are three netfilter fixes for net-next, fixing:
- the NAT issue reported by Stephen, which was caused by inverted logic
in NF_HOOK_COND(), causing it to skip the POST_ROUTING hook invocation
- an assertion in ct_extend, caused by invalid ordering in ctnetlink
when setting up new conntracks. Additionally it is invalid to
attach helpers to existing conntracks, which is disabled by this
patch.
- an skb leak in nf_queue when userspace returns NF_STOLEN as verdict
Please apply or pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-next-2.6.git master
Thanks!
include/linux/netfilter.h | 5 +++--
net/netfilter/nf_conntrack_netlink.c | 22 +++++++++++-----------
net/netfilter/nf_queue.c | 2 +-
3 files changed, 15 insertions(+), 14 deletions(-)
Eric Dumazet (1):
netfilter: nf_queue: fix NF_STOLEN skb leak
Pablo Neira Ayuso (1):
netfilter: ctnetlink: fix creation of conntrack with helpers
Patrick McHardy (1):
netfilter: restore POST_ROUTING hook in NF_HOOK_COND
^ permalink raw reply
* netfilter 01/03: restore POST_ROUTING hook in NF_HOOK_COND
From: Patrick McHardy @ 2010-02-19 17:02 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100219170206.18096.12788.sendpatchset@x2.localnet>
commit 4bac6b180771f7ef5275b1a6d88e630ca3a3d6f0
Author: Patrick McHardy <kaber@trash.net>
Date: Fri Feb 19 08:03:28 2010 +0100
netfilter: restore POST_ROUTING hook in NF_HOOK_COND
Commit 2249065 ("netfilter: get rid of the grossness in netfilter.h")
inverted the logic for conditional hook invocation, breaking the
POST_ROUTING hook invoked by ip_output().
Correct the logic and remove an unnecessary initialization.
Reported-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 7007945..89341c3 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -212,8 +212,9 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
struct net_device *in, struct net_device *out,
int (*okfn)(struct sk_buff *), bool cond)
{
- int ret = 1;
- if (cond ||
+ int ret;
+
+ if (!cond ||
(ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
ret = okfn(skb);
return ret;
^ permalink raw reply related
* Re: [RFC 1/2] sctp: convert hash list to RCU
From: Stephen Hemminger @ 2010-02-19 16:18 UTC (permalink / raw)
To: paulmck
Cc: Vlad Yasevich, Sridhar Samudrala, David S. Miller, netdev,
linux-sctp
In-Reply-To: <20100219155825.GA6778@linux.vnet.ibm.com>
On Fri, 19 Feb 2010 07:58:25 -0800
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> On Thu, Feb 18, 2010 at 09:55:21PM -0800, Stephen Hemminger wrote:
> > This patch converts existing SCTP hash list to using RCU
> > rather than reader/writer lock. Also, get rid of no longer used
> > locking wrappers.
> >
> > In future, the SCTP hash locking should be broken out from the
> > hash structure because of the wasted space for the hash locks
> > and associated holes. A single lock per hashlist is sufficient
> > now that RCU is used.
> >
> > Compile tested only. I can't think of an SCTP stress application.
> >
> > P.s: Some janitor ought to go through and remove the locking
> > macros here.
>
> One question below about what looks to be mixing of RCU and RCU-bh
> read-side critical sections while waiting only for RCU grace periods.
> Unless I am missing something, this can result in memory corruption.
>
Thanks, I copied the original locking which was broken there as well.
--
^ permalink raw reply
* Re: [RFC 1/2] sctp: convert hash list to RCU
From: Paul E. McKenney @ 2010-02-19 15:58 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Vlad Yasevich, Sridhar Samudrala, David S. Miller, netdev,
linux-sctp
In-Reply-To: <20100219055628.436258223@vyatta.com>
On Thu, Feb 18, 2010 at 09:55:21PM -0800, Stephen Hemminger wrote:
> This patch converts existing SCTP hash list to using RCU
> rather than reader/writer lock. Also, get rid of no longer used
> locking wrappers.
>
> In future, the SCTP hash locking should be broken out from the
> hash structure because of the wasted space for the hash locks
> and associated holes. A single lock per hashlist is sufficient
> now that RCU is used.
>
> Compile tested only. I can't think of an SCTP stress application.
>
> P.s: Some janitor ought to go through and remove the locking
> macros here.
One question below about what looks to be mixing of RCU and RCU-bh
read-side critical sections while waiting only for RCU grace periods.
Unless I am missing something, this can result in memory corruption.
Thanx, Paul
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
> --- a/include/net/sctp/sctp.h 2010-02-18 09:44:42.664390403 -0800
> +++ b/include/net/sctp/sctp.h 2010-02-18 09:52:48.004478538 -0800
> @@ -206,10 +206,6 @@ extern struct kmem_cache *sctp_bucket_ca
> #define sctp_local_bh_enable() local_bh_enable()
> #define sctp_spin_lock(lock) spin_lock(lock)
> #define sctp_spin_unlock(lock) spin_unlock(lock)
> -#define sctp_write_lock(lock) write_lock(lock)
> -#define sctp_write_unlock(lock) write_unlock(lock)
> -#define sctp_read_lock(lock) read_lock(lock)
> -#define sctp_read_unlock(lock) read_unlock(lock)
>
> /* sock lock wrappers. */
> #define sctp_lock_sock(sk) lock_sock(sk)
> @@ -649,6 +645,9 @@ static inline int sctp_vtag_hashfn(__u16
> #define sctp_for_each_hentry(epb, node, head) \
> hlist_for_each_entry(epb, node, head, node)
>
> +#define sctp_for_each_hentry_rcu(epb, node, head) \
> + hlist_for_each_entry_rcu(epb, node, head, node)
> +
> /* Is a socket of this style? */
> #define sctp_style(sk, style) __sctp_style((sk), (SCTP_SOCKET_##style))
> static inline int __sctp_style(const struct sock *sk, sctp_socket_type_t style)
> --- a/include/net/sctp/structs.h 2010-02-18 09:47:29.964390311 -0800
> +++ b/include/net/sctp/structs.h 2010-02-18 09:57:26.792896287 -0800
> @@ -111,7 +111,7 @@ struct sctp_bind_hashbucket {
>
> /* Used for hashing all associations. */
> struct sctp_hashbucket {
> - rwlock_t lock;
> + spinlock_t lock;
> struct hlist_head chain;
> } __attribute__((__aligned__(8)));
>
> @@ -1371,6 +1371,8 @@ struct sctp_endpoint {
> /* SCTP-AUTH: endpoint shared keys */
> struct list_head endpoint_shared_keys;
> __u16 active_key_id;
> +
> + struct rcu_head rcu;
> };
>
> /* Recover the outter endpoint structure. */
> --- a/net/sctp/endpointola.c 2010-02-18 09:43:22.868887786 -0800
> +++ b/net/sctp/endpointola.c 2010-02-18 10:00:55.807210206 -0800
> @@ -247,9 +247,9 @@ void sctp_endpoint_free(struct sctp_endp
> }
>
> /* Final destructor for endpoint. */
> -static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
> +static void sctp_endpoint_destroy_rcu(struct rcu_head *rcu)
> {
> - SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
> + struct sctp_endpoint *ep = container_of(rcu, struct sctp_endpoint, rcu);
>
> /* Free up the HMAC transform. */
> crypto_free_hash(sctp_sk(ep->base.sk)->hmac);
> @@ -286,6 +286,13 @@ static void sctp_endpoint_destroy(struct
> }
> }
>
> +static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
> +{
> + SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
> + call_rcu(&ep->rcu, sctp_endpoint_destroy_rcu);
> +}
> +
> +
> /* Hold a reference to an endpoint. */
> void sctp_endpoint_hold(struct sctp_endpoint *ep)
> {
> @@ -338,8 +345,9 @@ static struct sctp_association *__sctp_e
>
> hash = sctp_assoc_hashfn(ep->base.bind_addr.port, rport);
> head = &sctp_assoc_hashtable[hash];
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> asoc = sctp_assoc(epb);
> if (asoc->ep != ep || rport != asoc->peer.port)
> goto next;
> @@ -352,7 +360,7 @@ static struct sctp_association *__sctp_e
> next:
> asoc = NULL;
> }
> - read_unlock(&head->lock);
> + rcu_read_unlock();
> return asoc;
> }
>
> --- a/net/sctp/input.c 2010-02-18 09:51:39.104889498 -0800
> +++ b/net/sctp/input.c 2010-02-18 10:02:42.972601804 -0800
> @@ -704,9 +704,9 @@ static void __sctp_hash_endpoint(struct
> epb->hashent = sctp_ep_hashfn(epb->bind_addr.port);
> head = &sctp_ep_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - hlist_add_head(&epb->node, &head->chain);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_add_head_rcu(&epb->node, &head->chain);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Add an endpoint to the hash. Local BH-safe. */
> @@ -732,9 +732,9 @@ static void __sctp_unhash_endpoint(struc
>
> head = &sctp_ep_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - __hlist_del(&epb->node);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_del_rcu(&epb->node);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Remove endpoint from the hash. Local BH-safe. */
> @@ -756,8 +756,8 @@ static struct sctp_endpoint *__sctp_rcv_
>
> hash = sctp_ep_hashfn(ntohs(laddr->v4.sin_port));
> head = &sctp_ep_hashtable[hash];
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> + rcu_read_lock();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> ep = sctp_ep(epb);
> if (sctp_endpoint_is_match(ep, laddr))
> goto hit;
> @@ -767,7 +767,7 @@ static struct sctp_endpoint *__sctp_rcv_
>
> hit:
> sctp_endpoint_hold(ep);
> - read_unlock(&head->lock);
> + rcu_read_unlock();
> return ep;
> }
>
> @@ -784,9 +784,9 @@ static void __sctp_hash_established(stru
>
> head = &sctp_assoc_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - hlist_add_head(&epb->node, &head->chain);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_add_head_rcu(&epb->node, &head->chain);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Add an association to the hash. Local BH-safe. */
> @@ -813,9 +813,9 @@ static void __sctp_unhash_established(st
>
> head = &sctp_assoc_hashtable[epb->hashent];
>
> - sctp_write_lock(&head->lock);
> - __hlist_del(&epb->node);
> - sctp_write_unlock(&head->lock);
> + sctp_spin_lock(&head->lock);
> + hlist_del_rcu(&epb->node);
> + sctp_spin_unlock(&head->lock);
> }
>
> /* Remove association from the hash table. Local BH-safe. */
> @@ -847,22 +847,23 @@ static struct sctp_association *__sctp_l
> */
> hash = sctp_assoc_hashfn(ntohs(local->v4.sin_port), ntohs(peer->v4.sin_port));
> head = &sctp_assoc_hashtable[hash];
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> asoc = sctp_assoc(epb);
> transport = sctp_assoc_is_match(asoc, local, peer);
> if (transport)
> goto hit;
> }
>
> - read_unlock(&head->lock);
> + rcu_read_unlock();
>
> return NULL;
>
> hit:
> *pt = transport;
> sctp_association_hold(asoc);
> - read_unlock(&head->lock);
> + rcu_read_unlock();
> return asoc;
> }
>
> --- a/net/sctp/protocol.c 2010-02-18 09:42:04.556389428 -0800
> +++ b/net/sctp/protocol.c 2010-02-18 09:53:03.360663641 -0800
> @@ -1204,7 +1204,7 @@ SCTP_STATIC __init int sctp_init(void)
> goto err_ahash_alloc;
> }
> for (i = 0; i < sctp_assoc_hashsize; i++) {
> - rwlock_init(&sctp_assoc_hashtable[i].lock);
> + spin_lock_init(&sctp_assoc_hashtable[i].lock);
> INIT_HLIST_HEAD(&sctp_assoc_hashtable[i].chain);
> }
>
> @@ -1218,7 +1218,7 @@ SCTP_STATIC __init int sctp_init(void)
> goto err_ehash_alloc;
> }
> for (i = 0; i < sctp_ep_hashsize; i++) {
> - rwlock_init(&sctp_ep_hashtable[i].lock);
> + spin_lock_init(&sctp_ep_hashtable[i].lock);
> INIT_HLIST_HEAD(&sctp_ep_hashtable[i].chain);
> }
>
> --- a/net/sctp/proc.c 2010-02-18 10:03:50.428764115 -0800
> +++ b/net/sctp/proc.c 2010-02-18 10:05:09.961659526 -0800
> @@ -208,9 +208,9 @@ static int sctp_eps_seq_show(struct seq_
> return -ENOMEM;
>
> head = &sctp_ep_hashtable[hash];
> - sctp_local_bh_disable();
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock_bh();
Why _bh() here instead of just rcu_read_lock()? Either way, we would
need a call_rcu_bh() to wait for this particular RCU read-side critical
section -- call_rcu() won't necessarily do it because and RCU grace
period is not guaranteed to wait for RCU-bh read-side critical sections.
If we do need _bh() here, one approach would be to do call_rcu(), and
make the callback do call_rcu_bh(), and the _bh callback could then do
the free. This approach would be guaranteed to wait for both RCU and
RCU-bh read-side critical sections.
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> ep = sctp_ep(epb);
> sk = epb->sk;
> seq_printf(seq, "%8p %8p %-3d %-3d %-4d %-5d %5d %5lu ", ep, sk,
> @@ -221,8 +221,7 @@ static int sctp_eps_seq_show(struct seq_
> sctp_seq_dump_local_addrs(seq, epb);
> seq_printf(seq, "\n");
> }
> - read_unlock(&head->lock);
> - sctp_local_bh_enable();
> + rcu_read_unlock_bh();
>
> return 0;
> }
> @@ -312,9 +311,9 @@ static int sctp_assocs_seq_show(struct s
> return -ENOMEM;
>
> head = &sctp_assoc_hashtable[hash];
> - sctp_local_bh_disable();
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock_bh();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> assoc = sctp_assoc(epb);
> sk = epb->sk;
> seq_printf(seq,
> @@ -339,8 +338,7 @@ static int sctp_assocs_seq_show(struct s
> assoc->rtx_data_chunks);
> seq_printf(seq, "\n");
> }
> - read_unlock(&head->lock);
> - sctp_local_bh_enable();
> + rcu_read_unlock_bh();
>
> return 0;
> }
> @@ -425,9 +423,9 @@ static int sctp_remaddr_seq_show(struct
> return -ENOMEM;
>
> head = &sctp_assoc_hashtable[hash];
> - sctp_local_bh_disable();
> - read_lock(&head->lock);
> - sctp_for_each_hentry(epb, node, &head->chain) {
> +
> + rcu_read_lock_bh();
> + sctp_for_each_hentry_rcu(epb, node, &head->chain) {
> assoc = sctp_assoc(epb);
> list_for_each_entry(tsp, &assoc->peer.transport_addr_list,
> transports) {
> @@ -475,9 +473,7 @@ static int sctp_remaddr_seq_show(struct
> seq_printf(seq, "\n");
> }
> }
> -
> - read_unlock(&head->lock);
> - sctp_local_bh_enable();
> + rcu_read_unlock_bh();
>
> return 0;
>
>
> --
>
^ permalink raw reply
* [PATCH] tc35815: Remove a wrong netif_wake_queue() call which triggers BUG_ON
From: Atsushi Nemoto @ 2010-02-19 15:13 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
The netif_wake_queue() is called correctly (i.e. only on !txfull
condition) from txdone routine. So Unconditional call to the
netif_wake_queue() here is wrong. This might cause calling of
start_xmit routine on txfull state and trigger BUG_ON.
This bug does not happen when NAPI disabled. After txdone there
must be at least one free tx slot. But with NAPI, this is not
true anymore and the BUG_ON can hits on heavy load.
In this driver NAPI was enabled on 2.6.33-rc1 so this is
regression from 2.6.32 kernel.
Reported-by: Ralf Roesch <ralf.roesch@rw-gmbh.de>
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/tc35815.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index 75a669d..d71c197 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -1437,7 +1437,6 @@ static int tc35815_do_interrupt(struct net_device *dev, u32 status, int limit)
/* Transmit complete. */
lp->lstats.tx_ints++;
tc35815_txdone(dev);
- netif_wake_queue(dev);
if (ret < 0)
ret = 0;
}
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH] tc35815: Remove a wrong netif_wake_queue() call which triggers BUG_ON
From: Atsushi Nemoto @ 2010-02-19 15:31 UTC (permalink / raw)
To: netdev; +Cc: davem, ralf.roesch
In-Reply-To: <1266592438-10772-1-git-send-email-anemo@mba.ocn.ne.jp>
On Sat, 20 Feb 2010 00:13:58 +0900, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> @@ -1437,7 +1437,6 @@ static int tc35815_do_interrupt(struct net_device *dev, u32 status, int limit)
> /* Transmit complete. */
> lp->lstats.tx_ints++;
> tc35815_txdone(dev);
> - netif_wake_queue(dev);
> if (ret < 0)
> ret = 0;
> }
BTY, it looks isa-skelton.c have same problem.
void net_tx(struct net_device *dev)
{
...
if (netif_queue_stopped(dev) && ! tx_full(dev))
netif_wake_queue(dev);
...
}
static irqreturn_t net_interrupt(int irq, void *dev_id)
{
...
if (status & TX_INTR) {
/* Transmit complete. */
net_tx(dev);
np->stats.tx_packets++;
netif_wake_queue(dev);
}
...
}
I suppose here is the far origin of the bug :)
---
Atsushi Nemoto
^ permalink raw reply
* Re: [PATCH] greth: some driver cleanups
From: Jiri Pirko @ 2010-02-19 15:25 UTC (permalink / raw)
To: kirjanov; +Cc: davem, kristoffer, netdev
In-Reply-To: <20100219150051.GA12393@coldcone>
Fri, Feb 19, 2010 at 04:00:52PM CET, kirjanov@gmail.com wrote:
>On Fri, Feb 19, 2010 at 13:51 +0100, Jiri Pirko wrote:
>>
>> <snip>
>> >>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
>> >> return;
>> >> }
>> >>
>> >>- if (dev->mc_count == 0) {
>> >>+ if (!netdev_mc_count(dev)) {
>> also please use netdev_mc_empty() here.
>Some driver cleanups:
>* convert to use phy_find_first/phy_direct_connect
>* convert to use netdev_mc_* helpers
>* fixed missing validate_addr hook
>* removed netdev_priv castings
>
>Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
>---
>
> drivers/net/greth.c | 49 ++++++++++++++++++-------------------------------
> 1 files changed, 18 insertions(+), 31 deletions(-)
>
>diff --git a/drivers/net/greth.c b/drivers/net/greth.c
>index 457da1c..d203233 100644
>--- a/drivers/net/greth.c
>+++ b/drivers/net/greth.c
>@@ -965,7 +965,7 @@ static int greth_set_mac_add(struct net_device *dev, void *p)
> struct greth_private *greth;
> struct greth_regs *regs;
>
>- greth = (struct greth_private *) netdev_priv(dev);
>+ greth = netdev_priv(dev);
> regs = (struct greth_regs *) greth->regs;
>
> if (!is_valid_ether_addr(addr->sa_data))
>@@ -988,20 +988,14 @@ static u32 greth_hash_get_index(__u8 *addr)
> static void greth_set_hash_filter(struct net_device *dev)
> {
> struct dev_mc_list *curr;
>- struct greth_private *greth = (struct greth_private *) netdev_priv(dev);
>+ struct greth_private *greth = netdev_priv(dev);
> struct greth_regs *regs = (struct greth_regs *) greth->regs;
> u32 mc_filter[2];
>- unsigned int i, bitnr;
>+ unsigned int bitnr;
>
> mc_filter[0] = mc_filter[1] = 0;
>
>- curr = dev->mc_list;
>-
>- for (i = 0; i < dev->mc_count; i++, curr = curr->next) {
>-
>- if (!curr)
>- break; /* unexpected end of list */
>-
>+ netdev_for_each_mc_addr(curr, dev) {
> bitnr = greth_hash_get_index(curr->dmi_addr);
> mc_filter[bitnr >> 5] |= 1 << (bitnr & 31);
> }
>@@ -1031,7 +1025,7 @@ static void greth_set_multicast_list(struct net_device *dev)
> return;
> }
>
>- if (dev->mc_count == 0) {
>+ if (netdev_mc_empty(dev)) {
> cfg &= ~GRETH_CTRL_MCEN;
> GRETH_REGSAVE(regs->control, cfg);
> return;
>@@ -1160,6 +1154,7 @@ static struct net_device_ops greth_netdev_ops = {
> .ndo_stop = greth_close,
> .ndo_start_xmit = greth_start_xmit,
> .ndo_set_mac_address = greth_set_mac_add,
>+ .ndo_validate_addr = eth_validate_addr,
> };
>
> static inline int wait_for_mdio(struct greth_private *greth)
>@@ -1275,28 +1270,26 @@ static int greth_mdio_probe(struct net_device *dev)
> {
> struct greth_private *greth = netdev_priv(dev);
> struct phy_device *phy = NULL;
>- u32 interface;
>- int i;
>+ int ret;
>
> /* Find the first PHY */
>- for (i = 0; i < PHY_MAX_ADDR; i++) {
>- if (greth->mdio->phy_map[i]) {
>- phy = greth->mdio->phy_map[i];
>- break;
>- }
>- }
>+ phy = phy_find_first(greth->mdio);
>+
> if (!phy) {
> if (netif_msg_probe(greth))
> dev_err(&dev->dev, "no PHY found\n");
> return -ENXIO;
> }
>
>- if (greth->gbit_mac)
>- interface = PHY_INTERFACE_MODE_GMII;
>- else
>- interface = PHY_INTERFACE_MODE_MII;
>-
>- phy = phy_connect(dev, dev_name(&phy->dev), &greth_link_change, 0, interface);
>+ ret = phy_connect_direct(dev, phy, &greth_link_change,
>+ 0, greth->gbit_mac ?
>+ PHY_INTERFACE_MODE_GMII :
>+ PHY_INTERFACE_MODE_MII);
>+ if (ret) {
>+ if (netif_msg_ifup(greth))
>+ dev_err(&dev->dev, "could not attach to PHY\n");
>+ return ret;
>+ }
>
> if (greth->gbit_mac)
> phy->supported &= PHY_GBIT_FEATURES;
>@@ -1305,12 +1298,6 @@ static int greth_mdio_probe(struct net_device *dev)
>
> phy->advertising = phy->supported;
>
>- if (IS_ERR(phy)) {
>- if (netif_msg_ifup(greth))
>- dev_err(&dev->dev, "could not attach to PHY\n");
>- return PTR_ERR(phy);
>- }
>-
> greth->link = 0;
> greth->speed = 0;
> greth->duplex = -1;
Looks good.
Reviewed-by: Jiri Pirko <jpirko@redhat.com>
^ permalink raw reply
* Re: [PATCH 2/2] packet: convert socket list to RCU
From: Eric Dumazet @ 2010-02-19 15:16 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Li Zefan, netdev, linux-kernel
In-Reply-To: <20100219054201.501571315@vyatta.com>
Le jeudi 18 février 2010 à 21:41 -0800, Stephen Hemminger a écrit :
> Convert AF_PACKET to use RCU, eliminating one more reader/writer lock.
>
> I needed to create some minor additional socket list RCU infrastructure
> to make this work. Note: there is no need for a real sk_del_node_init_rcu(),
> because sk_del_node_init is doing the equivalent thing to
> hlst_del_init_rcu already; but added some comments to try and make that obvious.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
Stephen, I am a bit worried by the interaction between packet_release()
and packet_notifier()
With your version, packet_notifier() can run and let another cpu run
packet_release() un-contented. Both cpus could manipulate same po (and
particularly po->running)
Before your patch, the read_lock() done in packet_notifier() was
preventing packet_release() runnning at the same time.
Maybe packet_release() should lock po->bind_lock before manipulating
po->running, avoiding a refcount error.
Something like this preliminary patch :
[PATCH] packet: fix a race in packet_release
packet_release() has a potential race with packet_notifier(NETDEV_DOWN),
leading to a double __sock_put(). (dev_remove_pack() is safe)
Fix is to always use po->bind_lock before accessing po->running
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 10f7295..b706031 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1271,15 +1271,15 @@ static int packet_release(struct socket *sock)
* Unhook packet receive handler.
*/
+ spin_lock(&po->bind_lock);
if (po->running) {
- /*
- * Remove the protocol hook
- */
- dev_remove_pack(&po->prot_hook);
+ __sock_put(sk);
po->running = 0;
po->num = 0;
- __sock_put(sk);
- }
+ spin_unlock(&po->bind_lock);
+ dev_remove_pack(&po->prot_hook);
+ } else
+ spin_unlock(&po->bind_lock);
packet_flush_mclist(sk);
^ permalink raw reply related
* Re: [PATCH] greth: some driver cleanups
From: Denis Kirjanov <kirjanov@gmail.com @ 2010-02-19 15:00 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, kristoffer, netdev
In-Reply-To: <20100219125148.GA6598@psychotron.redhat.com>
On Fri, Feb 19, 2010 at 13:51 +0100, Jiri Pirko wrote:
>
> <snip>
> >>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
> >> return;
> >> }
> >>
> >>- if (dev->mc_count == 0) {
> >>+ if (!netdev_mc_count(dev)) {
> also please use netdev_mc_empty() here.
Some driver cleanups:
* convert to use phy_find_first/phy_direct_connect
* convert to use netdev_mc_* helpers
* fixed missing validate_addr hook
* removed netdev_priv castings
Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
---
drivers/net/greth.c | 49 ++++++++++++++++++-------------------------------
1 files changed, 18 insertions(+), 31 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 457da1c..d203233 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -965,7 +965,7 @@ static int greth_set_mac_add(struct net_device *dev, void *p)
struct greth_private *greth;
struct greth_regs *regs;
- greth = (struct greth_private *) netdev_priv(dev);
+ greth = netdev_priv(dev);
regs = (struct greth_regs *) greth->regs;
if (!is_valid_ether_addr(addr->sa_data))
@@ -988,20 +988,14 @@ static u32 greth_hash_get_index(__u8 *addr)
static void greth_set_hash_filter(struct net_device *dev)
{
struct dev_mc_list *curr;
- struct greth_private *greth = (struct greth_private *) netdev_priv(dev);
+ struct greth_private *greth = netdev_priv(dev);
struct greth_regs *regs = (struct greth_regs *) greth->regs;
u32 mc_filter[2];
- unsigned int i, bitnr;
+ unsigned int bitnr;
mc_filter[0] = mc_filter[1] = 0;
- curr = dev->mc_list;
-
- for (i = 0; i < dev->mc_count; i++, curr = curr->next) {
-
- if (!curr)
- break; /* unexpected end of list */
-
+ netdev_for_each_mc_addr(curr, dev) {
bitnr = greth_hash_get_index(curr->dmi_addr);
mc_filter[bitnr >> 5] |= 1 << (bitnr & 31);
}
@@ -1031,7 +1025,7 @@ static void greth_set_multicast_list(struct net_device *dev)
return;
}
- if (dev->mc_count == 0) {
+ if (netdev_mc_empty(dev)) {
cfg &= ~GRETH_CTRL_MCEN;
GRETH_REGSAVE(regs->control, cfg);
return;
@@ -1160,6 +1154,7 @@ static struct net_device_ops greth_netdev_ops = {
.ndo_stop = greth_close,
.ndo_start_xmit = greth_start_xmit,
.ndo_set_mac_address = greth_set_mac_add,
+ .ndo_validate_addr = eth_validate_addr,
};
static inline int wait_for_mdio(struct greth_private *greth)
@@ -1275,28 +1270,26 @@ static int greth_mdio_probe(struct net_device *dev)
{
struct greth_private *greth = netdev_priv(dev);
struct phy_device *phy = NULL;
- u32 interface;
- int i;
+ int ret;
/* Find the first PHY */
- for (i = 0; i < PHY_MAX_ADDR; i++) {
- if (greth->mdio->phy_map[i]) {
- phy = greth->mdio->phy_map[i];
- break;
- }
- }
+ phy = phy_find_first(greth->mdio);
+
if (!phy) {
if (netif_msg_probe(greth))
dev_err(&dev->dev, "no PHY found\n");
return -ENXIO;
}
- if (greth->gbit_mac)
- interface = PHY_INTERFACE_MODE_GMII;
- else
- interface = PHY_INTERFACE_MODE_MII;
-
- phy = phy_connect(dev, dev_name(&phy->dev), &greth_link_change, 0, interface);
+ ret = phy_connect_direct(dev, phy, &greth_link_change,
+ 0, greth->gbit_mac ?
+ PHY_INTERFACE_MODE_GMII :
+ PHY_INTERFACE_MODE_MII);
+ if (ret) {
+ if (netif_msg_ifup(greth))
+ dev_err(&dev->dev, "could not attach to PHY\n");
+ return ret;
+ }
if (greth->gbit_mac)
phy->supported &= PHY_GBIT_FEATURES;
@@ -1305,12 +1298,6 @@ static int greth_mdio_probe(struct net_device *dev)
phy->advertising = phy->supported;
- if (IS_ERR(phy)) {
- if (netif_msg_ifup(greth))
- dev_err(&dev->dev, "could not attach to PHY\n");
- return PTR_ERR(phy);
- }
-
greth->link = 0;
greth->speed = 0;
greth->duplex = -1;
^ permalink raw reply related
* [PATCH 1/1] add net fec phy lan8700
From: Carsten Behling @ 2010-02-19 14:21 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: kernel@pengutronix.de, dmalek@jlc.net
Signed-off-by: Carsten Behling <carsten.behling@garz-fricke.com>
---
Add support for lan8700 PHY.
drivers/net/fec.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 16a1d58..f739aca 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -1128,6 +1128,29 @@ static phy_info_t phy_info_dp83848= {
},
};
+static phy_info_t phy_info_lan8700 = {
+ 0x00007C0C,
+ "LAN8700",
+
+ (const phy_cmd_t []) { /* config */
+ { mk_mii_read(MII_REG_CR), mii_parse_cr },
+ { mk_mii_read(MII_REG_ANAR), mii_parse_anar },
+ { mk_mii_end, }
+ },
+ (const phy_cmd_t []) { /* startup - enable interrupts */
+ { mk_mii_write(MII_REG_CR, 0x1200), NULL }, /* autonegotiate */
+
+ { mk_mii_read(MII_REG_SR), mii_parse_sr },
+ { mk_mii_end, }
+ },
+ (const phy_cmd_t []) { /* ack_int - never happens, no interrupt */
+ { mk_mii_end, }
+ },
+ (const phy_cmd_t []) { /* shutdown */
+ { mk_mii_end, }
+ },
+};
+
/* ------------------------------------------------------------------------- */
static phy_info_t const * const phy_info[] = {
@@ -1137,6 +1160,7 @@ static phy_info_t const * const phy_info[] = {
&phy_info_am79c874,
&phy_info_ks8721bl,
&phy_info_dp83848,
+ &phy_info_lan8700,
NULL
};
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH net-next-2.6] vhost: Restart tx poll when socket send queue is full
From: Michael S. Tsirkin @ 2010-02-19 14:42 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: David Miller, netdev
In-Reply-To: <1266544807.15681.259.camel@w-sridhar.beaverton.ibm.com>
On Thu, Feb 18, 2010 at 06:00:07PM -0800, Sridhar Samudrala wrote:
> On Fri, 2010-02-19 at 00:30 +0200, Michael S. Tsirkin wrote:
> > On Thu, Feb 18, 2010 at 12:59:11PM -0800, Sridhar Samudrala wrote:
> > > When running guest to remote host TCP stream test using vhost-net
> > > via tap/macvtap, i am seeing network transmit hangs. This happens
> > > when handle_tx() returns because of the socket send queue full
> > > condition.
> > > This patch fixes this by restarting tx poll when hitting this
> > > condition.
> >
> >
> > Thanks! I would like to better understand what happens exactly.
> > Some questions below:
> >
> > >
> > > Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
> > >
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index 91a324c..82d4bbe 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -113,12 +113,16 @@ static void handle_tx(struct vhost_net *net)
> > > if (!sock)
> > > return;
> > >
> > > - wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> > > - if (wmem >= sock->sk->sk_sndbuf)
> > > - return;
> > > -
> >
> > The disadvantage here is that a spurious wakeup
> > when queue is still full becomes more expensive.
> >
> > > use_mm(net->dev.mm);
> > > mutex_lock(&vq->mutex);
> > > +
> > > + wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> > > + if (wmem >= sock->sk->sk_sndbuf) {
> > > + tx_poll_start(net, sock);
> >
> > Hmm. We already do
> > if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
> > tx_poll_start(net, sock);
> > set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> > break;
> > }
> > why does not this code trigger here?
>
> This check is done only when the ring is empty(head == vq->num).
> But we are breaking out of the loop here.
> if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
> vhost_poll_queue(&vq->poll);
> break;
> }
>
> I guess tx_poll_start() is missing here. The following patch fixes
> the hang and may be a better fix.
>
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 4c89283..fe9d296 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -172,6 +172,7 @@ static void handle_tx(struct vhost_net *net)
> vhost_add_used_and_signal(&net->dev, vq, head, 0);
> total_len += len;
> if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
> + tx_poll_start(net, sock);
> vhost_poll_queue(&vq->poll);
> break;
> }
>
> Thanks
> Sridhar
Hmm, this happens when
we have polled a lot of packets, and want to
give another vq a chance to poll.
Looks like a strange place to add it.
--
MST
^ permalink raw reply
* Re: [PATCH 2/5] netlink: store MSG_CMSG_COMPAT flag in netlink_skb_parms
From: Florian Westphal @ 2010-02-19 13:34 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev
In-Reply-To: <1266583764.3991.56.camel@jlt3.sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2010-02-19 at 13:41 +0100, Florian Westphal wrote:
> > From: Florian Westphal <fw@strlen.de>
> >
> > This allows the netlink processing context to determine if the data
> > needs any 32 bit fixups.
> >
> > Cc: Johannes Berg <johannes@sipsolutions.net>
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> > changes since v1: change msg_compat to bool.
> >
> > include/linux/netlink.h | 1 +
> > net/netlink/af_netlink.c | 3 +++
> > 2 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> > index fde27c0..c094694 100644
> > --- a/include/linux/netlink.h
> > +++ b/include/linux/netlink.h
> > @@ -164,6 +164,7 @@ struct netlink_skb_parms {
> > __u32 loginuid; /* Login (audit) uid */
> > __u32 sessionid; /* Session id (audit) */
> > __u32 sid; /* SELinux security id */
> > + bool msg_compat; /* Message needs 32bit fixups */
> > };
> >
> > #define NETLINK_CB(skb) (*(struct netlink_skb_parms*)&((skb)->cb))
> > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> > index 4910031..5ff97cf 100644
> > --- a/net/netlink/af_netlink.c
> > +++ b/net/netlink/af_netlink.c
> > @@ -1328,6 +1328,9 @@ static int netlink_sendmsg(struct kiocb *kiocb,
> > struct socket *sock,
> > NETLINK_CB(skb).dst_group = dst_group;
> > NETLINK_CB(skb).loginuid = audit_get_loginuid(current);
> > NETLINK_CB(skb).sessionid = audit_get_sessionid(current);
> > +#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
> > + NETLINK_CB(skb).msg_compat = !!(msg->msg_flags & MSG_CMSG_COMPAT);
> > +#endif
>
> Can't really decide ... does that ifdef make sense? Or should the
> variable be ifdef'ed as well?
The define is there because the assignment is only relevant for
COMPAT_NETLINK_MESSAGES=y.
I did not add the define to netlink.h because I don't see any advantage.
But I do not mind changing this.
^ permalink raw reply
* Re: kernel stack trace using conntrack
From: Patrick McHardy @ 2010-02-19 13:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Pablo Neira Ayuso, Ramblewski David,
netfilter-devel@vger.kernel.org, netdev
In-Reply-To: <1266582824.3136.1.camel@edumazet-laptop>
Eric Dumazet wrote:
> Le vendredi 19 février 2010 à 03:18 +0100, Pablo Neira Ayuso a écrit :
>> I have slightly tested the following patch here. I think it should fix
>> the problem.
>>
>> We can revisit ctnetlink_change_helper() later, I think there's some
>> code there that can be refactorized.
>>
>> Let me know if you're OK with it.
>
> I sucessfuly tested your patch Pablo, thanks.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH] greth: some driver cleanups
From: Jiri Pirko @ 2010-02-19 12:51 UTC (permalink / raw)
To: kirjanov; +Cc: davem, kristoffer, netdev
In-Reply-To: <20100219120425.GD2644@psychotron.redhat.com>
<snip>
>>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
>> return;
>> }
>>
>>- if (dev->mc_count == 0) {
>>+ if (!netdev_mc_count(dev)) {
also please use netdev_mc_empty() here.
^ permalink raw reply
* Wide variety of innovative treatments for hair loss and thinning hair.
From: Unknown, @ 2010-02-19 8:18 UTC (permalink / raw)
To: netdev
Pending delivery http://bespopc.crisivren.com/
^ 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