* Re: [PATCH RFC] tun: experimental zero copy tx support
From: Shirley Ma @ 2012-05-14 17:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, Stephen Hemminger, Joe Perches, Jason Wang,
netdev, linux-kernel, Ian.Campbell, kvm
In-Reply-To: <20120513155206.GA26847@redhat.com>
On Sun, 2012-05-13 at 18:52 +0300, Michael S. Tsirkin wrote:
> Let vhost-net utilize zero copy tx when the experimental
> zero copy mode is enabled and when used with tun. This works on
> top of the patchset 'copy aside frags with destructors' that I posted
> previously. This is not using tcp so doesn't have the
> issue with early skb cloning noticed by Ian.
>
> For those that wish to test this with kvm, I intend to post a patchset
> +
> git tree with just the necessary bits from the destructor patch
> a bit later.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Hello Mike,
Have you tested this patch? I think the difference between macvtap and
tap is tap forwarding the packet to bridge. The zerocopy is disabled in
this case.
Shirley
^ permalink raw reply
* Re: [PATCH] net: codel: fix build errors
From: Stephen Hemminger @ 2012-05-14 17:35 UTC (permalink / raw)
To: Sasha Levin; +Cc: edumazet, dave.taht, davem, linux-kernel, netdev
In-Reply-To: <1337015687-17693-1-git-send-email-levinsasha928@gmail.com>
On Mon, 14 May 2012 19:14:47 +0200
Sasha Levin <levinsasha928@gmail.com> wrote:
> Fix the following build error:
>
> net/sched/sch_fq_codel.c: In function 'fq_codel_dump_stats':
> net/sched/sch_fq_codel.c:464:3: error: unknown field 'qdisc_stats' specified in initializer
> net/sched/sch_fq_codel.c:464:3: warning: missing braces around initializer
> net/sched/sch_fq_codel.c:464:3: warning: (near initialization for 'st.<anonymous>')
> net/sched/sch_fq_codel.c:465:3: error: unknown field 'qdisc_stats' specified in initializer
> net/sched/sch_fq_codel.c:465:3: warning: excess elements in struct initializer
> net/sched/sch_fq_codel.c:465:3: warning: (near initialization for 'st')
> net/sched/sch_fq_codel.c:466:3: error: unknown field 'qdisc_stats' specified in initializer
> net/sched/sch_fq_codel.c:466:3: warning: excess elements in struct initializer
> net/sched/sch_fq_codel.c:466:3: warning: (near initialization for 'st')
> net/sched/sch_fq_codel.c:467:3: error: unknown field 'qdisc_stats' specified in initializer
> net/sched/sch_fq_codel.c:467:3: warning: excess elements in struct initializer
> net/sched/sch_fq_codel.c:467:3: warning: (near initialization for 'st')
> make[1]: *** [net/sched/sch_fq_codel.o] Error 1
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
Which Gcc version, looks like your compiler is old.
^ permalink raw reply
* Re: [net-next 06/12] ixgbe: Hardware Timestamping + PTP Hardware Clock (PHC)
From: Jacob Keller @ 2012-05-14 17:28 UTC (permalink / raw)
To: Richard Cochran
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <20120512053458.GC2190@netboy.at.omicron.at>
On 05/11/2012 10:34 PM, Richard Cochran wrote:
> On Fri, May 11, 2012 at 07:23:44PM +0000, Keller, Jacob E wrote:
>>
>>
>> I believe this very rare case might be possible, but I don't think
>> that checking the ptp seqid will fix anything. In normal cases,
>> hardware latches Rx packet timestamp, then the ptp packet goes into
>> the queue and we process it shortly after. Before we process that
>> packet there will never be another packet in the queue that needs a
>> timestamp. We know this because the hardware stops timestamping
>> until we unlatch the RX registers. This should mean we don't need to
>> check the sequence ID, and spending time doing it would never fix
>> the issue you are talking about.
>>
>> The issue is for when a packet is timestamped and then never reaches
>> the queue. Then the rx stamp registers are locked for good, because
>> we never clear them, and hardware would never timestamp another
>> receive packet. I don't know a good solution to this, except to
>> clear the registers periodically. Do you have any suggestions?
>
> Well, one solution would be to check every received packet with the
> BPF in ptp_classify.h (whenever Rx time stamping is enabled).
>
> When the driver finds an event packet in the Rx queue, and
> TSYNCRXCTL[RXTT] is set, it reads out the time stamp along with
> RXSATRL/H. If the fields match, then add the time stamp to the skb.
>
> [ Or perhaps instead of using RXSATRL/H, just use the descriptor bit.
> If *not* set, then the time stamp does not belong to this packet. ]
>
> HTH,
> Richard
Ok, this sounds like a good plan. Considering that the device already
doesn't allow timestamping of other types of packets, so it doesn't need
to be general purpose.
Am I correct in thinking all I need to do is check the type and if it
matches the currently configured rx timestamp mode, then double check
the bit for whether a timestamp is available, and whether the descriptor
had a timestamp bit enabled?
Thanks
- Jake
^ permalink raw reply
* [PATCH] net: codel: fix build errors
From: Sasha Levin @ 2012-05-14 17:14 UTC (permalink / raw)
To: edumazet, dave.taht, davem; +Cc: linux-kernel, netdev, Sasha Levin
Fix the following build error:
net/sched/sch_fq_codel.c: In function 'fq_codel_dump_stats':
net/sched/sch_fq_codel.c:464:3: error: unknown field 'qdisc_stats' specified in initializer
net/sched/sch_fq_codel.c:464:3: warning: missing braces around initializer
net/sched/sch_fq_codel.c:464:3: warning: (near initialization for 'st.<anonymous>')
net/sched/sch_fq_codel.c:465:3: error: unknown field 'qdisc_stats' specified in initializer
net/sched/sch_fq_codel.c:465:3: warning: excess elements in struct initializer
net/sched/sch_fq_codel.c:465:3: warning: (near initialization for 'st')
net/sched/sch_fq_codel.c:466:3: error: unknown field 'qdisc_stats' specified in initializer
net/sched/sch_fq_codel.c:466:3: warning: excess elements in struct initializer
net/sched/sch_fq_codel.c:466:3: warning: (near initialization for 'st')
net/sched/sch_fq_codel.c:467:3: error: unknown field 'qdisc_stats' specified in initializer
net/sched/sch_fq_codel.c:467:3: warning: excess elements in struct initializer
net/sched/sch_fq_codel.c:467:3: warning: (near initialization for 'st')
make[1]: *** [net/sched/sch_fq_codel.o] Error 1
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
include/linux/pkt_sched.h | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 32aef0a..c304eda 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -732,7 +732,9 @@ struct tc_fq_codel_xstats {
union {
struct tc_fq_codel_qd_stats qdisc_stats;
struct tc_fq_codel_cl_stats class_stats;
- };
+ } u;
+#define qdisc_stats u.qdisc_stats
+#define class_stats u.class_stats
};
#endif
--
1.7.8.6
^ permalink raw reply related
* Re: [PATCH RFC] tun: experimental zero copy tx support
From: Michael S. Tsirkin @ 2012-05-14 17:04 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Joe Perches, Jason Wang, netdev, linux-kernel,
Ian.Campbell, kvm
In-Reply-To: <20120514095446.3ce307d2@nehalam.linuxnetplumber.net>
On Mon, May 14, 2012 at 09:54:46AM -0700, Stephen Hemminger wrote:
> On Sun, 13 May 2012 18:52:06 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > + /* Userspace may produce vectors with count greater than
> > + * MAX_SKB_FRAGS, so we need to linearize parts of the skb
> > + * to let the rest of data to be fit in the frags.
> > + */
> Rather than complex partial code, just go through slow path for
> requests with too many frags (or for really small requests).
> Creating mixed skb's seems too easy to get wrong.
I don't object in principle but macvtap has same code
so seems better to stay consistent.
--
MST
^ permalink raw reply
* Re: [PATCH RFC] tun: experimental zero copy tx support
From: Stephen Hemminger @ 2012-05-14 16:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, Joe Perches, Jason Wang, netdev, linux-kernel,
Ian.Campbell, kvm
In-Reply-To: <20120513155206.GA26847@redhat.com>
On Sun, 13 May 2012 18:52:06 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> + /* Userspace may produce vectors with count greater than
> + * MAX_SKB_FRAGS, so we need to linearize parts of the skb
> + * to let the rest of data to be fit in the frags.
> + */
Rather than complex partial code, just go through slow path for
requests with too many frags (or for really small requests).
Creating mixed skb's seems too easy to get wrong.
^ permalink raw reply
* RE: pch_gbe: oops with vlan (resolved)
From: Andy Cress @ 2012-05-14 16:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1336774254.31653.287.camel@edumazet-glaptop>
Eric,
This patch resolved the driver oops (transmit queue timeout) with vlan. It makes a lot of sense that the wrong spin_locks caused this.
Thanks a lot.
Andy
-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Eric Dumazet
Sent: Friday, May 11, 2012 6:11 PM
To: Andy Cress
Cc: netdev
Subject: Re: pch_gbe: oops with vlan (new)
On Fri, 2012-05-11 at 23:13 +0200, Eric Dumazet wrote:
> On Fri, 2012-05-11 at 13:48 -0700, Andy Cress wrote:
> > Folks,
> >
> > I am looking for help in debugging a pch_gbe driver oops/abort.
> >
> > Kernel: version 2.6.32-220.el6.i686 (RHEL6.2)
> > Driver: pch_gbe version 0.91-NAPI (source tarball we used is at
> > https://sendfile.kontron.com/message/24tdUi6MXklnUtBLnOsumq until May
> > 16)
> > NIC: 0b:00.1 Ethernet controller [0200]: Intel Corporation Platform
> > Controller Hub EG20T Gigabit Ethernet Controller [8086:8802] (rev 02)
> >
> > Configuration, with VLAN:
> > eth0 (not started)
> > eth0.100 = 192.168.100.1
> > eth0.200 = 192.168.200.1
> > eth0.6 = 192.168.6.1
> >
> > When starting the VLAN configuration, then doing a ping test for >= 5
> > minutes, I get a kernel oop/abort message as shown below. This does not
> > happen without configuring VLAN.
> > Where should I look for possible causes for a transmit queue timeout
> > like this?
> >
> > I have contacted the OKI/LAPIS driver authors, but no response so far.
> > I thought that this group might be able to comment from similar
> > experiences.
> >
> > Andy
>
> typical sign of a buggy driver
>
> A quick look in current Linus tree show a non existent synchronization
> between ndo_start_xmit and TX completion.
>
> tx completion uses a tx_queue_lock spinlock for nothing but false sense
> of correctness.
Please try the following patch : (based on current net-next tree)
Also this driver has a strange RX path : It does a copy of incoming
frames on fixed size skbs, (2048+overhead -> kmalloc-4096 pool) instead
of using skb of the right size...
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 2
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 25 ++++------
2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 9f3dbc4..b07311e 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -584,7 +584,6 @@ struct pch_gbe_hw_stats {
/**
* struct pch_gbe_adapter - board specific private data structure
* @stats_lock: Spinlock structure for status
- * @tx_queue_lock: Spinlock structure for transmit
* @ethtool_lock: Spinlock structure for ethtool
* @irq_sem: Semaphore for interrupt
* @netdev: Pointer of network device structure
@@ -609,7 +608,6 @@ struct pch_gbe_hw_stats {
struct pch_gbe_adapter {
spinlock_t stats_lock;
- spinlock_t tx_queue_lock;
spinlock_t ethtool_lock;
atomic_t irq_sem;
struct net_device *netdev;
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 9dc7e50..3787c64 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -645,14 +645,11 @@ static void pch_gbe_mac_set_pause_packet(struct pch_gbe_hw *hw)
*/
static int pch_gbe_alloc_queues(struct pch_gbe_adapter *adapter)
{
- int size;
-
- size = (int)sizeof(struct pch_gbe_tx_ring);
- adapter->tx_ring = kzalloc(size, GFP_KERNEL);
+ adapter->tx_ring = kzalloc(sizeof(*adapter->tx_ring), GFP_KERNEL);
if (!adapter->tx_ring)
return -ENOMEM;
- size = (int)sizeof(struct pch_gbe_rx_ring);
- adapter->rx_ring = kzalloc(size, GFP_KERNEL);
+
+ adapter->rx_ring = kzalloc(sizeof(*adapter->rx_ring), GFP_KERNEL);
if (!adapter->rx_ring) {
kfree(adapter->tx_ring);
return -ENOMEM;
@@ -1169,7 +1166,6 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
struct sk_buff *tmp_skb;
unsigned int frame_ctrl;
unsigned int ring_num;
- unsigned long flags;
/*-- Set frame control --*/
frame_ctrl = 0;
@@ -1216,14 +1212,14 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
}
}
}
- spin_lock_irqsave(&tx_ring->tx_lock, flags);
+
ring_num = tx_ring->next_to_use;
if (unlikely((ring_num + 1) == tx_ring->count))
tx_ring->next_to_use = 0;
else
tx_ring->next_to_use = ring_num + 1;
- spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
+
buffer_info = &tx_ring->buffer_info[ring_num];
tmp_skb = buffer_info->skb;
@@ -1525,7 +1521,7 @@ pch_gbe_alloc_rx_buffers_pool(struct pch_gbe_adapter *adapter,
&rx_ring->rx_buff_pool_logic,
GFP_KERNEL);
if (!rx_ring->rx_buff_pool) {
- pr_err("Unable to allocate memory for the receive poll buffer\n");
+ pr_err("Unable to allocate memory for the receive pool buffer\n");
return -ENOMEM;
}
memset(rx_ring->rx_buff_pool, 0, size);
@@ -1644,15 +1640,17 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
pr_debug("called pch_gbe_unmap_and_free_tx_resource() %d count\n",
cleaned_count);
/* Recover from running out of Tx resources in xmit_frame */
+ spin_lock(&tx_ring->tx_lock);
if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev)))) {
netif_wake_queue(adapter->netdev);
adapter->stats.tx_restart_count++;
pr_debug("Tx wake queue\n");
}
- spin_lock(&adapter->tx_queue_lock);
+
tx_ring->next_to_clean = i;
- spin_unlock(&adapter->tx_queue_lock);
+
pr_debug("next_to_clean : %d\n", tx_ring->next_to_clean);
+ spin_unlock(&tx_ring->tx_lock);
return cleaned;
}
@@ -2043,7 +2041,6 @@ static int pch_gbe_sw_init(struct pch_gbe_adapter *adapter)
return -ENOMEM;
}
spin_lock_init(&adapter->hw.miim_lock);
- spin_lock_init(&adapter->tx_queue_lock);
spin_lock_init(&adapter->stats_lock);
spin_lock_init(&adapter->ethtool_lock);
atomic_set(&adapter->irq_sem, 0);
@@ -2148,10 +2145,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
tx_ring->next_to_use, tx_ring->next_to_clean);
return NETDEV_TX_BUSY;
}
- spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
/* CRC,ITAG no support */
pch_gbe_tx_queue(adapter, tx_ring, skb);
+ spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
return NETDEV_TX_OK;
}
--
To unsubscribe from this list: send the line "unsubscribe netdev" 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 related
* [PATCH net-next v4] be2net: Fix to allow get/set of debug levels in the firmware.
From: Somnath Kotur @ 2012-05-14 16:29 UTC (permalink / raw)
To: netdev, bhutchings; +Cc: Somnath Kotur, Suresh Reddy
Fixed missing paranthesis warning
Incorporated review comments by Ben Hutchings.
Signed-off-by: Suresh Reddy <suresh.reddy@emulex.com>
Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
---
drivers/net/ethernet/emulex/benet/be.h | 3 +
drivers/net/ethernet/emulex/benet/be_cmds.c | 56 +++++++++++++++++
drivers/net/ethernet/emulex/benet/be_cmds.h | 57 +++++++++++++++++
drivers/net/ethernet/emulex/benet/be_ethtool.c | 77 ++++++++++++++++++++++++
drivers/net/ethernet/emulex/benet/be_main.c | 37 +++++++++++
5 files changed, 230 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index c3ee910..9817fed 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -415,6 +415,7 @@ struct be_adapter {
bool wol;
u32 max_pmac_cnt; /* Max secondary UC MACs programmable */
u32 uc_macs; /* Count of secondary UC MAC programmed */
+ u32 msg_enable;
};
#define be_physfn(adapter) (!adapter->is_virtfn)
@@ -603,4 +604,6 @@ extern void be_parse_stats(struct be_adapter *adapter);
extern int be_load_fw(struct be_adapter *adapter, u8 *func);
extern bool be_is_wol_supported(struct be_adapter *adapter);
extern bool be_pause_supported(struct be_adapter *adapter);
+extern u32 be_get_fw_log_level(struct be_adapter *adapter);
+
#endif /* BE_H */
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 43167e8..b24623c 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -2589,4 +2589,60 @@ err:
mutex_unlock(&adapter->mbox_lock);
pci_free_consistent(adapter->pdev, cmd.size, cmd.va, cmd.dma);
return status;
+
+}
+int be_cmd_get_ext_fat_capabilites(struct be_adapter *adapter,
+ struct be_dma_mem *cmd)
+{
+ struct be_mcc_wrb *wrb;
+ struct be_cmd_req_get_ext_fat_caps *req;
+ int status;
+
+ if (mutex_lock_interruptible(&adapter->mbox_lock))
+ return -1;
+
+ wrb = wrb_from_mbox(adapter);
+ if (!wrb) {
+ status = -EBUSY;
+ goto err;
+ }
+
+ req = cmd->va;
+ be_wrb_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
+ OPCODE_COMMON_GET_EXT_FAT_CAPABILITES,
+ cmd->size, wrb, cmd);
+ req->parameter_type = cpu_to_le32(1);
+
+ status = be_mbox_notify_wait(adapter);
+err:
+ mutex_unlock(&adapter->mbox_lock);
+ return status;
+}
+
+int be_cmd_set_ext_fat_capabilites(struct be_adapter *adapter,
+ struct be_dma_mem *cmd,
+ struct be_fat_conf_params *configs)
+{
+ struct be_mcc_wrb *wrb;
+ struct be_cmd_req_set_ext_fat_caps *req;
+ int status;
+
+ spin_lock_bh(&adapter->mcc_lock);
+
+ wrb = wrb_from_mccq(adapter);
+ if (!wrb) {
+ status = -EBUSY;
+ goto err;
+ }
+
+ req = cmd->va;
+ memcpy(&req->set_params, configs, sizeof(struct be_fat_conf_params));
+ be_wrb_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
+ OPCODE_COMMON_SET_EXT_FAT_CAPABILITES,
+ cmd->size, wrb, cmd);
+
+ status = be_mcc_notify_wait(adapter);
+err:
+ spin_unlock_bh(&adapter->mcc_lock);
+ return status;
}
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 944f031..0b1029b 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -189,6 +189,8 @@ struct be_mcc_mailbox {
#define OPCODE_COMMON_GET_PHY_DETAILS 102
#define OPCODE_COMMON_SET_DRIVER_FUNCTION_CAP 103
#define OPCODE_COMMON_GET_CNTL_ADDITIONAL_ATTRIBUTES 121
+#define OPCODE_COMMON_GET_EXT_FAT_CAPABILITES 125
+#define OPCODE_COMMON_SET_EXT_FAT_CAPABILITES 126
#define OPCODE_COMMON_GET_MAC_LIST 147
#define OPCODE_COMMON_SET_MAC_LIST 148
#define OPCODE_COMMON_GET_HSW_CONFIG 152
@@ -1602,6 +1604,56 @@ static inline void *be_erx_stats_from_cmd(struct be_adapter *adapter)
}
}
+
+/************** get fat capabilites *******************/
+#define MAX_MODULES 27
+#define MAX_MODES 4
+#define MODE_UART 0
+#define FW_LOG_LEVEL_DEFAULT 48
+#define FW_LOG_LEVEL_FATAL 64
+
+struct ext_fat_mode {
+ u8 mode;
+ u8 rsvd0;
+ u16 port_mask;
+ u32 dbg_lvl;
+ u64 fun_mask;
+} __packed;
+
+struct ext_fat_modules {
+ u8 modules_str[32];
+ u32 modules_id;
+ u32 num_modes;
+ struct ext_fat_mode trace_lvl[MAX_MODES];
+} __packed;
+
+struct be_fat_conf_params {
+ u32 max_log_entries;
+ u32 log_entry_size;
+ u8 log_type;
+ u8 max_log_funs;
+ u8 max_log_ports;
+ u8 rsvd0;
+ u32 supp_modes;
+ u32 num_modules;
+ struct ext_fat_modules module[MAX_MODULES];
+} __packed;
+
+struct be_cmd_req_get_ext_fat_caps {
+ struct be_cmd_req_hdr hdr;
+ u32 parameter_type;
+};
+
+struct be_cmd_resp_get_ext_fat_caps {
+ struct be_cmd_resp_hdr hdr;
+ struct be_fat_conf_params get_params;
+};
+
+struct be_cmd_req_set_ext_fat_caps {
+ struct be_cmd_req_hdr hdr;
+ struct be_fat_conf_params set_params;
+};
+
extern int be_pci_fnum_get(struct be_adapter *adapter);
extern int be_cmd_POST(struct be_adapter *adapter);
extern int be_cmd_mac_addr_query(struct be_adapter *adapter, u8 *mac_addr,
@@ -1707,4 +1759,9 @@ extern int be_cmd_set_hsw_config(struct be_adapter *adapter, u16 pvid,
extern int be_cmd_get_hsw_config(struct be_adapter *adapter, u16 *pvid,
u32 domain, u16 intf_id);
extern int be_cmd_get_acpi_wol_cap(struct be_adapter *adapter);
+extern int be_cmd_get_ext_fat_capabilites(struct be_adapter *adapter,
+ struct be_dma_mem *cmd);
+extern int be_cmd_set_ext_fat_capabilites(struct be_adapter *adapter,
+ struct be_dma_mem *cmd,
+ struct be_fat_conf_params *cfgs);
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 747f68f..88f1107 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -878,6 +878,81 @@ be_read_eeprom(struct net_device *netdev, struct ethtool_eeprom *eeprom,
return status;
}
+static u32 be_get_msg_level(struct net_device *netdev)
+{
+ struct be_adapter *adapter = netdev_priv(netdev);
+
+ if (lancer_chip(adapter)) {
+ dev_err(&adapter->pdev->dev, "Operation not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ return adapter->msg_enable;
+}
+
+static void be_set_fw_log_level(struct be_adapter *adapter, u32 level)
+{
+ struct be_dma_mem extfat_cmd;
+ struct be_fat_conf_params *cfgs;
+ int status;
+ int i, j;
+
+ memset(&extfat_cmd, 0, sizeof(struct be_dma_mem));
+ extfat_cmd.size = sizeof(struct be_cmd_resp_get_ext_fat_caps);
+ extfat_cmd.va = pci_alloc_consistent(adapter->pdev, extfat_cmd.size,
+ &extfat_cmd.dma);
+ if (!extfat_cmd.va) {
+ dev_err(&adapter->pdev->dev, "%s: Memory allocation failure\n",
+ __func__);
+ goto err;
+ }
+ status = be_cmd_get_ext_fat_capabilites(adapter, &extfat_cmd);
+ if (!status) {
+ cfgs = (struct be_fat_conf_params *)(extfat_cmd.va +
+ sizeof(struct be_cmd_resp_hdr));
+ for (i = 0; i < cfgs->num_modules; i++) {
+ for (j = 0; j < cfgs->module[i].num_modes; j++) {
+ if (cfgs->module[i].trace_lvl[j].mode ==
+ MODE_UART)
+ cfgs->module[i].trace_lvl[j].dbg_lvl =
+ cpu_to_le32(level);
+ }
+ }
+ status = be_cmd_set_ext_fat_capabilites(adapter, &extfat_cmd,
+ cfgs);
+ if (status)
+ dev_err(&adapter->pdev->dev,
+ "Message level set failed\n");
+ } else {
+ dev_err(&adapter->pdev->dev, "Message level get failed\n");
+ }
+
+ pci_free_consistent(adapter->pdev, extfat_cmd.size, extfat_cmd.va,
+ extfat_cmd.dma);
+err:
+ return;
+}
+
+static void be_set_msg_level(struct net_device *netdev, u32 level)
+{
+ struct be_adapter *adapter = netdev_priv(netdev);
+
+ if (lancer_chip(adapter)) {
+ dev_err(&adapter->pdev->dev, "Operation not supported\n");
+ return;
+ }
+
+ if (adapter->msg_enable == level)
+ return;
+
+ if ((level & NETIF_MSG_HW) != (adapter->msg_enable & NETIF_MSG_HW))
+ be_set_fw_log_level(adapter, level & NETIF_MSG_HW ?
+ FW_LOG_LEVEL_DEFAULT : FW_LOG_LEVEL_FATAL);
+ adapter->msg_enable = level;
+
+ return;
+}
+
const struct ethtool_ops be_ethtool_ops = {
.get_settings = be_get_settings,
.get_drvinfo = be_get_drvinfo,
@@ -893,6 +968,8 @@ const struct ethtool_ops be_ethtool_ops = {
.set_pauseparam = be_set_pauseparam,
.get_strings = be_get_stat_strings,
.set_phys_id = be_set_phys_id,
+ .get_msglevel = be_get_msg_level,
+ .set_msglevel = be_set_msg_level,
.get_sset_count = be_get_sset_count,
.get_ethtool_stats = be_get_ethtool_stats,
.get_regs_len = be_get_reg_len,
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 6d5d30b..c2286a2 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3367,9 +3367,43 @@ bool be_is_wol_supported(struct be_adapter *adapter)
!be_is_wol_excluded(adapter)) ? true : false;
}
+u32 be_get_fw_log_level(struct be_adapter *adapter)
+{
+ struct be_dma_mem extfat_cmd;
+ struct be_fat_conf_params *cfgs;
+ int status;
+ u32 level = 0;
+ int j;
+
+ memset(&extfat_cmd, 0, sizeof(struct be_dma_mem));
+ extfat_cmd.size = sizeof(struct be_cmd_resp_get_ext_fat_caps);
+ extfat_cmd.va = pci_alloc_consistent(adapter->pdev, extfat_cmd.size,
+ &extfat_cmd.dma);
+ if (!extfat_cmd.va) {
+ dev_err(&adapter->pdev->dev, "%s: Memory allocation failure\n",
+ __func__);
+ goto err;
+ }
+
+ status = be_cmd_get_ext_fat_capabilites(adapter, &extfat_cmd);
+ if (!status) {
+ cfgs = (struct be_fat_conf_params *)(extfat_cmd.va +
+ sizeof(struct be_cmd_resp_hdr));
+ for (j = 0; j < cfgs->module[0].num_modes; j++) {
+ if (cfgs->module[0].trace_lvl[j].mode == MODE_UART)
+ level = cfgs->module[0].trace_lvl[j].dbg_lvl;
+ }
+ }
+ pci_free_consistent(adapter->pdev, extfat_cmd.size, extfat_cmd.va,
+ extfat_cmd.dma);
+err:
+ return level;
+}
+
static int be_get_config(struct be_adapter *adapter)
{
int status;
+ u32 level;
status = be_cmd_query_fw_cfg(adapter, &adapter->port_num,
&adapter->function_mode, &adapter->function_caps);
@@ -3407,6 +3441,9 @@ static int be_get_config(struct be_adapter *adapter)
if (be_is_wol_supported(adapter))
adapter->wol = true;
+ level = be_get_fw_log_level(adapter);
+ adapter->msg_enable = level <= FW_LOG_LEVEL_DEFAULT ? NETIF_MSG_HW : 0;
+
return 0;
}
--
1.5.6.1
^ permalink raw reply related
* ppp/l2tp doing oversized allocations ?
From: Dave Jones @ 2012-05-14 16:29 UTC (permalink / raw)
To: netdev; +Cc: Fedora Kernel Team
We just got this trace from reported by a Fedora user running 3.3.4
:WARNING: at mm/page_alloc.c:2204 __alloc_pages_nodemask+0x231/0x8f0()
:Call Trace:
: [<ffffffff81057abf>] warn_slowpath_common+0x7f/0xc0
: [<ffffffff81057b1a>] warn_slowpath_null+0x1a/0x20
: [<ffffffff81129671>] __alloc_pages_nodemask+0x231/0x8f0
: [<ffffffff814e84db>] ? dev_queue_xmit+0x1db/0x640
: [<ffffffff8151f210>] ? ip_forward_options+0x1f0/0x1f0
: [<ffffffff814ef7a1>] ? neigh_direct_output+0x11/0x20
: [<ffffffff81520dee>] ? ip_finish_output+0x17e/0x2f0
: [<ffffffff8151f210>] ? ip_forward_options+0x1f0/0x1f0
: [<ffffffff811608d3>] alloc_pages_current+0xa3/0x110
: [<ffffffff811254b4>] __get_free_pages+0x14/0x50
: [<ffffffff8116b99f>] kmalloc_order_trace+0x3f/0xd0
: [<ffffffff8156d137>] ? xfrm4_output_finish+0x27/0x40
: [<ffffffff8116c8c7>] __kmalloc+0x177/0x1a0
: [<ffffffff81521196>] ? ip_queue_xmit+0x156/0x400
: [<ffffffff814dab07>] pskb_expand_head+0x87/0x310
: [<ffffffff8113dbf9>] ? __mod_zone_page_state+0x49/0x50
: [<ffffffffa05e84dd>] pppol2tp_xmit+0x1ed/0x220 [l2tp_ppp]
: [<ffffffffa05d3d5b>] ppp_push+0x15b/0x650 [ppp_generic]
: [<ffffffff814dacc4>] ? pskb_expand_head+0x244/0x310
: [<ffffffff811285ab>] ? free_compound_page+0x1b/0x20
: [<ffffffff8112cff3>] ? __put_compound_page+0x23/0x30
: [<ffffffff8112d175>] ? put_compound_page+0x125/0x1c0
: [<ffffffffa05d489f>] ppp_xmit_process+0x46f/0x660 [ppp_generic]
: [<ffffffffa05d4bc8>] ppp_start_xmit+0x138/0x1d0 [ppp_generic]
: [<ffffffff814e7f62>] dev_hard_start_xmit+0x332/0x6d0
: [<ffffffff81503e2a>] sch_direct_xmit+0xfa/0x1d0
: [<ffffffff81503fa6>] __qdisc_run+0xa6/0x130
: [<ffffffff814e84be>] dev_queue_xmit+0x1be/0x640
: [<ffffffff8151f210>] ? ip_forward_options+0x1f0/0x1f0
: [<ffffffff814ef7a1>] neigh_direct_output+0x11/0x20
: [<ffffffff81520dee>] ip_finish_output+0x17e/0x2f0
: [<ffffffff81521906>] ip_output+0x66/0xa0
: [<ffffffff81521002>] ? __ip_local_out+0xa2/0xb0
: [<ffffffff815792ae>] xfrm_output_resume+0x38e/0x3f0
: [<ffffffff8116c0db>] ? kfree+0x3b/0x150
: [<ffffffff81297a10>] ? cryptd_free+0x60/0x60
: [<ffffffffa04fc110>] esp_output_done+0x30/0x40 [esp4]
: [<ffffffffa05ed774>] authenc_request_complete+0x14/0x20 [authenc]
: [<ffffffffa05ede4e>] crypto_authenc_givencrypt_done+0x2e/0x40 [authenc]
: [<ffffffff8129728c>] cryptd_blkcipher_crypt+0x5c/0x70
: [<ffffffff812972dc>] cryptd_blkcipher_encrypt+0x1c/0x20
: [<ffffffff81297a66>] cryptd_queue_worker+0x56/0x80
: [<ffffffff810747ae>] process_one_work+0x11e/0x470
: [<ffffffff810755bf>] worker_thread+0x15f/0x360
: [<ffffffff81075460>] ? manage_workers+0x230/0x230
: [<ffffffff81079da3>] kthread+0x93/0xa0
: [<ffffffff815fd2a4>] kernel_thread_helper+0x4/0x10
: [<ffffffff81079d10>] ? kthread_freezable_should_stop+0x70/0x70
: [<ffffffff815fd2a0>] ? gs_change+0x13/0x13
That WARN statement is this in the page allocator..
2197 /*
2198 * In the slowpath, we sanity check order to avoid ever trying to
2199 * reclaim >= MAX_ORDER areas which will never succeed. Callers may
2200 * be using allocators in order of preference for an area that is
2201 * too large.
2202 */
2203 if (order >= MAX_ORDER) {
2204 WARN_ON_ONCE(!(gfp_mask & __GFP_NOWARN));
2205 return NULL;
2206 }
Dave
^ permalink raw reply
* Re: [PATCH] netfilter: xt_HMARK: endian bugs
From: Eric Dumazet @ 2012-05-14 16:24 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Jan Engelhardt, Pablo Neira Ayuso, kaber@trash.net,
jengelh@medozas.de, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, dan.carpenter@oracle.com,
hans@schillstrom.com
In-Reply-To: <201205141809.18174.hans.schillstrom@ericsson.com>
On Mon, 2012-05-14 at 18:09 +0200, Hans Schillstrom wrote:
> This context can contain both le & be machines,
> so at least in hmark it make sense
Before jhash() and its shuffle ? What do you mean ?
Please respin your patch using (__force u16/u32) instead of
useless/expensive ntohs() / ntohl() (in _this_ context of hashing)
If you compare two 32bits values, of course they must have same
ordering, but seeding jhash() is another matter.
(Granted all calls use the same ordering of course)
sparse is great tool, but if you add useless ntohl() calls to make
sparse silent, then its probably better to not use sparse.
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: notify userspace of security level change
From: valdis.kletnieks @ 2012-05-14 16:22 UTC (permalink / raw)
To: Gustavo Padovan
Cc: linville, davem, linux-wireless, linux-bluetooth, linux-kernel,
netdev
In-Reply-To: <1336890007-10646-1-git-send-email-gustavo@padovan.org>
[-- Attachment #1: Type: text/plain, Size: 571 bytes --]
On Sun, 13 May 2012 03:20:07 -0300, Gustavo Padovan said:
> It enables the userspace a security level change when the socket is
> already connected and create a way to notify the socket the result of the
> request. At the moment of the request the socket is made non writable, if
> the request fails the connections closes, otherwise the socket is made
> writable again, POLL_OUT is emmited.
What happens to in-flight data already written but not yet consumed? I'm
smelling a possible race condition with data sent under the old level but
read under the new level....
[-- Attachment #2: Type: application/pgp-signature, Size: 865 bytes --]
^ permalink raw reply
* Re: [PATCH] netfilter: xt_HMARK: endian bugs
From: Hans Schillstrom @ 2012-05-14 16:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jan Engelhardt, Pablo Neira Ayuso, kaber@trash.net,
jengelh@medozas.de, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, dan.carpenter@oracle.com,
hans@schillstrom.com
In-Reply-To: <1337009079.8512.535.camel@edumazet-glaptop>
On Monday 14 May 2012 17:24:39 Eric Dumazet wrote:
> On Mon, 2012-05-14 at 17:05 +0200, Jan Engelhardt wrote:
> > On Monday 2012-05-14 16:40, Pablo Neira Ayuso wrote:
> >
> > >> - if (t->uports.p16.dst < t->uports.p16.src)
> > >> + if (ntohs(t->uports.p16.dst) < ntohs(t->uports.p16.src))
> > >
> > >Do we really need this to make sparse happy?
> >
> > You need it to make *maths* happy.
> >
> > Consider
> >
> > 384 < 65407
> >
> > but
> >
> > ntohs(384) > ntohs(65407)
> > <=> 32769 > 32767
> > --
>
> Doesnt matter at all in this context.
This context can contain both le & be machines,
so at least in hmark it make sense
> Take a look at
>
> void __skb_get_rxhash(struct sk_buff *skb)
>
> if ((__force u16)keys.port16[1] < (__force u16)keys.port16[0])
> swap(...)
>
>
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* [PATCH v2 1/6] netfilter: sanity checks on NFPROTO_NUMPROTO
From: Alban Crequy @ 2012-05-14 16:04 UTC (permalink / raw)
To: Alban Crequy
Cc: Pablo Neira Ayuso, Patrick McHardy, Vincent Sanders,
Javier Martinez Canillas, netfilter-devel, netdev
In-Reply-To: <20120514163949.37e614f4@rainbow.cbg.collabora.co.uk>
Le Mon, 14 May 2012 16:39:49 +0100,
Alban Crequy <alban.crequy@collabora.co.uk> a écrit :
> Le Mon, 14 May 2012 16:42:35 +0200,
> Pablo Neira Ayuso <pablo@netfilter.org> a écrit :
>
> > On Mon, May 14, 2012 at 02:56:34PM +0100, Alban Crequy wrote:
> > > With the NFPROTO_* constants introduced by commit 7e9c6e
> > > ("netfilter: Introduce NFPROTO_* constants"), it is too easy to
> > > confuse PF_* and NFPROTO_* constants in new protocols.
> > >
> > > Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
> > > Reviewed-by: Javier Martinez Canillas
> > > <javier.martinez@collabora.co.uk> Reviewed-by: Vincent Sanders
> > > <vincent.sanders@collabora.co.uk> ---
> > > net/netfilter/core.c | 5 +++++
> > > 1 files changed, 5 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> > > index e1b7e05..4f16552 100644
> > > --- a/net/netfilter/core.c
> > > +++ b/net/netfilter/core.c
> > > @@ -67,6 +67,11 @@ int nf_register_hook(struct nf_hook_ops *reg)
> > > struct nf_hook_ops *elem;
> > > int err;
> > >
> > > + if (reg->pf >= NFPROTO_NUMPROTO || reg->hooknum >=
> > > NF_MAX_HOOKS) {
> > > + BUG();
> > > + return 1;
> >
> > nf_register_hook returns a negative value on error. -EINVAL can be
> > fine.
>
> Is it the patch you mean? Do you want me to do a series repost?
Please disregard the previous patch, this is the correct one.
From: Alban Crequy <alban.crequy@collabora.co.uk>
netfilter: sanity checks on NFPROTO_NUMPROTO
With the NFPROTO_* constants introduced by commit 7e9c6e ("netfilter: Introduce
NFPROTO_* constants"), it is too easy to confuse PF_* and NFPROTO_* constants
in new protocols.
Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
---
net/netfilter/core.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index e1b7e05..7422989 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -67,6 +67,14 @@ int nf_register_hook(struct nf_hook_ops *reg)
struct nf_hook_ops *elem;
int err;
+ if (reg->pf >= NFPROTO_NUMPROTO || reg->hooknum >= NF_MAX_HOOKS) {
+ WARN(reg->pf >= NFPROTO_NUMPROTO,
+ "netfilter: Invalid nfproto %d\n", reg->pf);
+ WARN(reg->hooknum >= NF_MAX_HOOKS,
+ "netfilter: Invalid hooknum %d\n", reg->hooknum);
+ return -EINVAL;
+ }
+
err = mutex_lock_interruptible(&nf_hook_mutex);
if (err < 0)
return err;
--
1.7.2.5
--
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 related
* Re: [PATCH] [RFC] netfilter: don't assume NFPROTO_* are like PF_*
From: Alban Crequy @ 2012-05-14 15:42 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Pablo Neira Ayuso, Patrick McHardy, Vincent Sanders,
Javier Martinez Canillas, netfilter-devel, netdev
In-Reply-To: <alpine.LNX.2.01.1205141707490.11548@frira.vanv.qr>
Le Mon, 14 May 2012 17:09:54 +0200 (CEST),
Jan Engelhardt <jengelh@inai.de> a écrit :
>
> On Monday 2012-05-14 15:58, Alban Crequy wrote:
> >--- a/include/linux/netfilter.h
> >+++ b/include/linux/netfilter.h
> >@@ -62,11 +62,11 @@ enum nf_inet_hooks {
> >
> > enum {
> > NFPROTO_UNSPEC = 0,
> >- NFPROTO_IPV4 = 2,
> >- NFPROTO_ARP = 3,
> >- NFPROTO_BRIDGE = 7,
> >- NFPROTO_IPV6 = 10,
> >- NFPROTO_DECNET = 12,
> >+ NFPROTO_IPV4,
> >+ NFPROTO_ARP,
> >+ NFPROTO_BRIDGE,
> >+ NFPROTO_IPV6,
> >+ NFPROTO_DECNET,
> > NFPROTO_NUMPROTO,
> > };
>
> This must not be changed under any circumstances. It is exported to
> and used by userspace. (Except perhaps for NFPROTO_DECNET, which
> refers to a quite dead protocol that I think no user parts have ever
> used NFPROTO_DECNET.) I would consider it acceptable to change the
> value for NFPROTO_DECNET if Pablo joins.
Thanks for the review. I didn't realize it was exported to and used by
userspace. I would drop this patch then.
Alban
^ permalink raw reply
* Re: [PATCH ethtool] Add command to dump module EEPROM
From: Stuart Hodgson @ 2012-05-14 15:36 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Yaniv Rosner, David Miller, netdev, Eilon Greenstein
In-Reply-To: <1337005139.2550.6.camel@bwh-desktop.uk.solarflarecom.com>
On 14/05/12 15:18, Ben Hutchings wrote:
> On Mon, 2012-05-14 at 18:13 +0300, Yaniv Rosner wrote:
>> Hi Ben,
>> This patch adds a new option to dump (SFP+, XFP, ...) module EEPROM following
>> recent support to kernel side. Below some examples:
>>
>> bash-3.00# ethtool -m eth1 offset 0x14 length 32 raw on
>> JDSU PLRXPLSCS432
>>
>> bash-3.00# ethtool -m eth1 offset 0x14 length 32
>> Offset Values
>> ------ ------
>> 0x0014 4a 44 53 55 20 20 20 20 20 20 20 20 20 20 20 20
>> 0x0024 00 00 01 9c 50 4c 52 58 50 4c 53 43 53 34 33 32
>>
>> Please consider applying to ethtool.
>
> I agree there should be ASCII-hex and binary dump modes, but we should
> also support decoding of recognised EEPROM types (as Stuart proposed
> earlier).
I have a patch to do this as well, but also parse the SFP+ EEPROM.
I need to fix it up after some of the changes to the patches that added
kernel support but can submit in the next day or so if this would be of
use.
Stu
^ permalink raw reply
* Re: [PATCH 1/6] netfilter: sanity checks on NFPROTO_NUMPROTO
From: Alban Crequy @ 2012-05-14 15:39 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Patrick McHardy, Vincent Sanders, Javier Martinez Canillas,
netfilter-devel, netdev
In-Reply-To: <20120514144235.GE12992@1984>
Le Mon, 14 May 2012 16:42:35 +0200,
Pablo Neira Ayuso <pablo@netfilter.org> a écrit :
> On Mon, May 14, 2012 at 02:56:34PM +0100, Alban Crequy wrote:
> > With the NFPROTO_* constants introduced by commit 7e9c6e
> > ("netfilter: Introduce NFPROTO_* constants"), it is too easy to
> > confuse PF_* and NFPROTO_* constants in new protocols.
> >
> > Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
> > Reviewed-by: Javier Martinez Canillas
> > <javier.martinez@collabora.co.uk> Reviewed-by: Vincent Sanders
> > <vincent.sanders@collabora.co.uk> ---
> > net/netfilter/core.c | 5 +++++
> > 1 files changed, 5 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> > index e1b7e05..4f16552 100644
> > --- a/net/netfilter/core.c
> > +++ b/net/netfilter/core.c
> > @@ -67,6 +67,11 @@ int nf_register_hook(struct nf_hook_ops *reg)
> > struct nf_hook_ops *elem;
> > int err;
> >
> > + if (reg->pf >= NFPROTO_NUMPROTO || reg->hooknum >=
> > NF_MAX_HOOKS) {
> > + BUG();
> > + return 1;
>
> nf_register_hook returns a negative value on error. -EINVAL can be
> fine.
Is it the patch you mean? Do you want me to do a series repost?
From: Alban Crequy <alban.crequy@collabora.co.uk>
netfilter: sanity checks on NFPROTO_NUMPROTO
With the NFPROTO_* constants introduced by commit 7e9c6e ("netfilter: Introduce
NFPROTO_* constants"), it is too easy to confuse PF_* and NFPROTO_* constants
in new protocols.
Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
---
net/netfilter/core.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index e1b7e05..ac56c5b 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -67,6 +67,11 @@ int nf_register_hook(struct nf_hook_ops *reg)
struct nf_hook_ops *elem;
int err;
+ if (reg->pf >= NFPROTO_NUMPROTO || reg->hooknum >= NF_MAX_HOOKS) {
+ WARN();
+ return -EINVAL;
+ }
+
err = mutex_lock_interruptible(&nf_hook_mutex);
if (err < 0)
return err;
--
1.7.2.5
--
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 related
* Re: [patch] Re: qlge driver corrupting kernel memory
From: Mike Galbraith @ 2012-05-14 15:33 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo; +Cc: netdev, Jitendra Kalsaria
In-Reply-To: <1336904179.7390.16.camel@marge.simpson.net>
(add CC)
On Sun, 2012-05-13 at 12:16 +0200, Mike Galbraith wrote:
> Erm, with a quilt refresh you get the compiling version :)
>
> glge: Fix double pci_free_consistent() upon tx_ring->q allocation failure
>
> Let ql_free_tx_resources() do it's job. You are not helping.
>
> Signed-off-by: Mike Galbraith <mgalbraith@suse.de>
> ---
> drivers/net/qlge/qlge_main.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> --- a/drivers/net/qlge/qlge_main.c
> +++ b/drivers/net/qlge/qlge_main.c
> @@ -2664,11 +2664,8 @@ static int ql_alloc_tx_resources(struct
> pci_alloc_consistent(qdev->pdev, tx_ring->wq_size,
> &tx_ring->wq_base_dma);
>
> - if ((tx_ring->wq_base == NULL) ||
> - tx_ring->wq_base_dma & WQ_ADDR_ALIGN) {
> - QPRINTK(qdev, IFUP, ERR, "tx_ring alloc failed.\n");
> - return -ENOMEM;
> - }
> + if ((tx_ring->wq_base == NULL) || tx_ring->wq_base_dma & WQ_ADDR_ALIGN)
> + goto err;
> tx_ring->q =
> kmalloc(tx_ring->wq_len * sizeof(struct tx_ring_desc), GFP_KERNEL);
> if (tx_ring->q == NULL)
> @@ -2676,8 +2673,7 @@ static int ql_alloc_tx_resources(struct
>
> return 0;
> err:
> - pci_free_consistent(qdev->pdev, tx_ring->wq_size,
> - tx_ring->wq_base, tx_ring->wq_base_dma);
> + QPRINTK(qdev, IFUP, ERR, "tx_ring alloc failed.\n");
> return -ENOMEM;
> }
>
>
>
^ permalink raw reply
* Re: [PATCH] netfilter: xt_HMARK: endian bugs
From: Eric Dumazet @ 2012-05-14 15:24 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Pablo Neira Ayuso, Hans Schillstrom, kaber, jengelh,
netfilter-devel, netdev, dan.carpenter, hans
In-Reply-To: <alpine.LNX.2.01.1205141702110.11548@frira.vanv.qr>
On Mon, 2012-05-14 at 17:05 +0200, Jan Engelhardt wrote:
> On Monday 2012-05-14 16:40, Pablo Neira Ayuso wrote:
>
> >> - if (t->uports.p16.dst < t->uports.p16.src)
> >> + if (ntohs(t->uports.p16.dst) < ntohs(t->uports.p16.src))
> >
> >Do we really need this to make sparse happy?
>
> You need it to make *maths* happy.
>
> Consider
>
> 384 < 65407
>
> but
>
> ntohs(384) > ntohs(65407)
> <=> 32769 > 32767
> --
Doesnt matter at all in this context.
Take a look at
void __skb_get_rxhash(struct sk_buff *skb)
if ((__force u16)keys.port16[1] < (__force u16)keys.port16[0])
swap(...)
^ permalink raw reply
* Re: [PATCH] [RFC] netfilter: don't assume NFPROTO_* are like PF_*
From: Jan Engelhardt @ 2012-05-14 15:09 UTC (permalink / raw)
To: Alban Crequy
Cc: Pablo Neira Ayuso, Patrick McHardy, Vincent Sanders,
Javier Martinez Canillas, netfilter-devel, netdev
In-Reply-To: <1337003909-2582-1-git-send-email-alban.crequy@collabora.co.uk>
On Monday 2012-05-14 15:58, Alban Crequy wrote:
>--- a/include/linux/netfilter.h
>+++ b/include/linux/netfilter.h
>@@ -62,11 +62,11 @@ enum nf_inet_hooks {
>
> enum {
> NFPROTO_UNSPEC = 0,
>- NFPROTO_IPV4 = 2,
>- NFPROTO_ARP = 3,
>- NFPROTO_BRIDGE = 7,
>- NFPROTO_IPV6 = 10,
>- NFPROTO_DECNET = 12,
>+ NFPROTO_IPV4,
>+ NFPROTO_ARP,
>+ NFPROTO_BRIDGE,
>+ NFPROTO_IPV6,
>+ NFPROTO_DECNET,
> NFPROTO_NUMPROTO,
> };
This must not be changed under any circumstances. It is exported to
and used by userspace. (Except perhaps for NFPROTO_DECNET, which
refers to a quite dead protocol that I think no user parts have ever
used NFPROTO_DECNET.) I would consider it acceptable to change the
value for NFPROTO_DECNET if Pablo joins.
^ permalink raw reply
* RE: [PATCH 2/6] netfilter: decnet: switch hook PFs to nfproto
From: Jan Engelhardt @ 2012-05-14 15:06 UTC (permalink / raw)
To: David Laight
Cc: Alban Crequy, Pablo Neira Ayuso, Patrick McHardy, Vincent Sanders,
Javier Martinez Canillas, netfilter-devel, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6F0A@saturn3.aculab.com>
On Monday 2012-05-14 16:18, David Laight wrote:
>
>> NFPROTO_* constants were usually equal to PF_* constants but it is not
>> necessary and it will waste less memory if we don't do so
>> (see commit 7e9c6e
>> "netfilter: Introduce NFPROTO_* constants")
>...
>>
>> static struct nf_hook_ops dnrmg_ops __read_mostly = {
>> .hook = dnrmg_hook,
>> - .pf = PF_DECnet,
>> + .pf = NFPROTO_DECNET,
>> .hooknum = NF_DN_ROUTE,
>> .priority = NF_DN_PRI_DNRTMSG,
>> };
>
>Might it be worth renaming the .pf member to (say) .nfproto
>to help avoid confusion?
Yes that seems like a sensible thing.
^ permalink raw reply
* Re: [PATCH] netfilter: xt_HMARK: endian bugs
From: Jan Engelhardt @ 2012-05-14 15:05 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Hans Schillstrom, kaber, jengelh, netfilter-devel, netdev,
dan.carpenter, hans
In-Reply-To: <20120514144052.GD12992@1984>
On Monday 2012-05-14 16:40, Pablo Neira Ayuso wrote:
>> - if (t->uports.p16.dst < t->uports.p16.src)
>> + if (ntohs(t->uports.p16.dst) < ntohs(t->uports.p16.src))
>
>Do we really need this to make sparse happy?
You need it to make *maths* happy.
Consider
384 < 65407
but
ntohs(384) > ntohs(65407)
<=> 32769 > 32767
^ permalink raw reply
* Re: [PATCH] netfilter: xt_HMARK: endian bugs
From: Hans Schillstrom @ 2012-05-14 15:05 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: kaber@trash.net, jengelh@medozas.de,
netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
dan.carpenter@oracle.com, hans@schillstrom.com
In-Reply-To: <20120514144052.GD12992@1984>
On Monday 14 May 2012 16:40:52 Pablo Neira Ayuso wrote:
> On Mon, May 14, 2012 at 03:42:23PM +0200, Hans Schillstrom wrote:
> > A mix of u32 and __be32 causes endian warning.
> > Switch to __be32 and __be16 for addresses and ports.
> > Added (__force u32) at some places.
> >
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> > ---
> > include/linux/netfilter/xt_HMARK.h | 4 ++--
> > net/netfilter/xt_HMARK.c | 35 ++++++++++++++++++-----------------
> > 2 files changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/include/linux/netfilter/xt_HMARK.h b/include/linux/netfilter/xt_HMARK.h
> > index abb1650..e2af67e 100644
> > --- a/include/linux/netfilter/xt_HMARK.h
> > +++ b/include/linux/netfilter/xt_HMARK.h
> > @@ -24,8 +24,8 @@ enum {
> >
> > union hmark_ports {
> > struct {
> > - __u16 src;
> > - __u16 dst;
> > + __be16 src;
> > + __be16 dst;
> > } p16;
> > __u32 v32;
> > };
> > diff --git a/net/netfilter/xt_HMARK.c b/net/netfilter/xt_HMARK.c
> > index 32fbd73..38ed442 100644
> > --- a/net/netfilter/xt_HMARK.c
> > +++ b/net/netfilter/xt_HMARK.c
> > @@ -32,13 +32,13 @@ MODULE_ALIAS("ipt_HMARK");
> > MODULE_ALIAS("ip6t_HMARK");
> >
> > struct hmark_tuple {
> > - u32 src;
> > - u32 dst;
> > + __be32 src;
> > + __be32 dst;
> > union hmark_ports uports;
> > uint8_t proto;
> > };
> >
> > -static inline u32 hmark_addr6_mask(const __u32 *addr32, const __u32 *mask)
> > +static inline __be32 hmark_addr6_mask(const __be32 *addr32, const __be32 *mask)
> > {
> > return (addr32[0] & mask[0]) ^
> > (addr32[1] & mask[1]) ^
> > @@ -46,8 +46,8 @@ static inline u32 hmark_addr6_mask(const __u32 *addr32, const __u32 *mask)
> > (addr32[3] & mask[3]);
> > }
> >
> > -static inline u32
> > -hmark_addr_mask(int l3num, const __u32 *addr32, const __u32 *mask)
> > +static inline __be32
> > +hmark_addr_mask(int l3num, const __be32 *addr32, const __be32 *mask)
> > {
> > switch (l3num) {
> > case AF_INET:
> > @@ -74,10 +74,10 @@ hmark_ct_set_htuple(const struct sk_buff *skb, struct hmark_tuple *t,
> > otuple = &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
> > rtuple = &ct->tuplehash[IP_CT_DIR_REPLY].tuple;
> >
> > - t->src = hmark_addr_mask(otuple->src.l3num, otuple->src.u3.all,
> > - info->src_mask.all);
> > - t->dst = hmark_addr_mask(otuple->src.l3num, rtuple->src.u3.all,
> > - info->dst_mask.all);
> > + t->src = hmark_addr_mask(otuple->src.l3num, otuple->src.u3.ip6,
> > + info->src_mask.ip6);
> > + t->dst = hmark_addr_mask(otuple->src.l3num, rtuple->src.u3.ip6,
> > + info->dst_mask.ip6);
> >
> > if (info->flags & XT_HMARK_FLAG(XT_HMARK_METHOD_L3))
> > return 0;
> > @@ -88,7 +88,7 @@ hmark_ct_set_htuple(const struct sk_buff *skb, struct hmark_tuple *t,
> > t->uports.p16.dst = rtuple->src.u.all;
> > t->uports.v32 = (t->uports.v32 & info->port_mask.v32) |
> > info->port_set.v32;
> > - if (t->uports.p16.dst < t->uports.p16.src)
> > + if (ntohs(t->uports.p16.dst) < ntohs(t->uports.p16.src))
>
> Do we really need this to make sparse happy?
__force is ok for Sparse,
but I realize that the mixing little and big endian machines will not work
>
> > swap(t->uports.p16.dst, t->uports.p16.src);
> > }
> >
> > @@ -103,10 +103,11 @@ hmark_hash(struct hmark_tuple *t, const struct xt_hmark_info *info)
> > {
> > u32 hash;
> >
> > - if (t->dst < t->src)
> > + if (ntohl(t->dst) < ntohl(t->src))
> > swap(t->src, t->dst);
> >
> > - hash = jhash_3words(t->src, t->dst, t->uports.v32, info->hashrnd);
> > + hash = jhash_3words((__force u32) t->src, (__force u32) t->dst,
> > + t->uports.v32, info->hashrnd);
> > hash = hash ^ (t->proto & info->proto_mask);
> >
> > return (hash % info->hmodulus) + info->hoffset;
>
> This will clash with my patch. No problem, I'll manually fix it
> myself.
Thanks
>
> > @@ -129,7 +130,7 @@ hmark_set_tuple_ports(const struct sk_buff *skb, unsigned int nhoff,
> > t->uports.v32 = (t->uports.v32 & info->port_mask.v32) |
> > info->port_set.v32;
> >
> > - if (t->uports.p16.dst < t->uports.p16.src)
> > + if (ntohs(t->uports.p16.dst) < ntohs(t->uports.p16.src))
> > swap(t->uports.p16.dst, t->uports.p16.src);
> > }
> >
> > @@ -178,8 +179,8 @@ hmark_pkt_set_htuple_ipv6(const struct sk_buff *skb, struct hmark_tuple *t,
> > return -1;
> > }
> > noicmp:
> > - t->src = hmark_addr6_mask(ip6->saddr.s6_addr32, info->src_mask.all);
> > - t->dst = hmark_addr6_mask(ip6->daddr.s6_addr32, info->dst_mask.all);
> > + t->src = hmark_addr6_mask(ip6->saddr.s6_addr32, info->src_mask.ip6);
> > + t->dst = hmark_addr6_mask(ip6->daddr.s6_addr32, info->dst_mask.ip6);
> >
> > if (info->flags & XT_HMARK_FLAG(XT_HMARK_METHOD_L3))
> > return 0;
> > @@ -255,8 +256,8 @@ hmark_pkt_set_htuple_ipv4(const struct sk_buff *skb, struct hmark_tuple *t,
> > }
> > }
> >
> > - t->src = (__force u32) ip->saddr;
> > - t->dst = (__force u32) ip->daddr;
> > + t->src = ip->saddr;
> > + t->dst = ip->daddr;
> >
> > t->src &= info->src_mask.ip;
> > t->dst &= info->dst_mask.ip;
> > --
> > 1.7.2.3
> >
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* Re: [PATCH 1/6] netfilter: sanity checks on NFPROTO_NUMPROTO
From: Jan Engelhardt @ 2012-05-14 15:00 UTC (permalink / raw)
To: Alban Crequy
Cc: Pablo Neira Ayuso, Patrick McHardy, Vincent Sanders,
Javier Martinez Canillas, netfilter-devel, netdev
In-Reply-To: <1337003799-2517-1-git-send-email-alban.crequy@collabora.co.uk>
On Monday 2012-05-14 15:56, Alban Crequy wrote:
>With the NFPROTO_* constants introduced by commit 7e9c6e ("netfilter: Introduce
>NFPROTO_* constants"), it is too easy to confuse PF_* and NFPROTO_* constants
>in new protocols.
>index e1b7e05..4f16552 100644
>--- a/net/netfilter/core.c
>+++ b/net/netfilter/core.c
>@@ -67,6 +67,11 @@ int nf_register_hook(struct nf_hook_ops *reg)
> struct nf_hook_ops *elem;
> int err;
>
>+ if (reg->pf >= NFPROTO_NUMPROTO || reg->hooknum >= NF_MAX_HOOKS) {
>+ BUG();
>+ return 1;
>+ }
Like always, I'd prefer a WARN() instead, here paired with return -EINVAL.
Especially when the error path is (seems) simple, halting the entire machine
does not look very nice.
^ permalink raw reply
* Re: [PATCH 1/4] netfilter: ipset: fix timeout value overflow bug
From: Eric Dumazet @ 2012-05-14 14:47 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: David Laight, netfilter-devel, davem, netdev, Jozsef Kadlecsik
In-Reply-To: <20120514143635.GA12992@1984>
On Mon, 2012-05-14 at 16:36 +0200, Pablo Neira Ayuso wrote:
> On Mon, May 14, 2012 at 03:19:49PM +0100, David Laight wrote:
> >
> > > --- a/include/linux/netfilter/ipset/ip_set_timeout.h
> > > +++ b/include/linux/netfilter/ipset/ip_set_timeout.h
> > > @@ -30,6 +30,10 @@ ip_set_timeout_uget(struct nlattr *tb)
> > > {
> > > unsigned int timeout = ip_set_get_h32(tb);
> > >
> > > + /* Normalize to fit into jiffies */
> > > + if (timeout > UINT_MAX/1000)
> > > + timeout = UINT_MAX/1000;
> > > +
> >
> > Doesn't that rather assume that HZ is 1000 ?
>
> Indeed. I overlooked that. Thanks David.
I dont think so.
1000 here is really MSEC_PER_SEC
(All occurrences should be changed in this file)
^ permalink raw reply
* Re: [PATCH 2/6] netfilter: decnet: switch hook PFs to nfproto
From: Pablo Neira Ayuso @ 2012-05-14 14:45 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Alban Crequy, Patrick McHardy, Vincent Sanders,
Javier Martinez Canillas, netfilter-devel, netdev
In-Reply-To: <1337003799-2517-2-git-send-email-alban.crequy@collabora.co.uk>
@Jan,
I remember you introduced all this NFPROTO_* thing time ago.
Any complain on this patchset?
Thanks.
On Mon, May 14, 2012 at 02:56:35PM +0100, Alban Crequy wrote:
> NFPROTO_* constants were usually equal to PF_* constants but it is not
> necessary and it will waste less memory if we don't do so (see commit 7e9c6e
> "netfilter: Introduce NFPROTO_* constants")
>
> Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reviewed-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
> ---
> net/decnet/netfilter/dn_rtmsg.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
> index 1531135..7fb7250 100644
> --- a/net/decnet/netfilter/dn_rtmsg.c
> +++ b/net/decnet/netfilter/dn_rtmsg.c
> @@ -118,7 +118,7 @@ static inline void dnrmg_receive_user_skb(struct sk_buff *skb)
>
> static struct nf_hook_ops dnrmg_ops __read_mostly = {
> .hook = dnrmg_hook,
> - .pf = PF_DECnet,
> + .pf = NFPROTO_DECNET,
> .hooknum = NF_DN_ROUTE,
> .priority = NF_DN_PRI_DNRTMSG,
> };
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" 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
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