* [net-next 03/15] i40e: fix whitespace
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Trivial whitespace fix.
Change-Id: Ib7c70891a33c4b3d200c69367549d0dbdee0f076
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 22fcf94..45bdcca 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -533,6 +533,7 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)
struct i40e_ring *tx_ring = ACCESS_ONCE(vsi->tx_rings[i]);
if (!tx_ring)
continue;
+
dev_info(&pf->pdev->dev,
" tx_rings[%i]: desc = %p\n",
i, tx_ring->desc);
--
1.8.3.1
^ permalink raw reply related
* [net-next 02/15] i40e: Fix SR-IOV VF port VLAN
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem; +Cc: Greg Rose, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Greg Rose <gregory.v.rose@intel.com>
This patch fixes two different problems.
1) The port VLAN configuration was not persistent across VF driver
loads and unloads.
2) The port VLAN configuration was only correct the first time it was
set. Switching the port VLAN on and off would cause subsequent VLAN
configurations to be corrupted in the VSI. Ensure that the correct
bits are being set for the VSI port VLAN configuration.
Change-Id: I7ebf5329f77eb8d73ccd3324eb346b3abeea737d
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +++++---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 16 ++++++++++++++--
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h | 1 +
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 55fd23d..d0b2046 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2016,8 +2016,9 @@ int i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
vsi->info.valid_sections = cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
vsi->info.pvid = cpu_to_le16(vid);
- vsi->info.port_vlan_flags |= I40E_AQ_VSI_PVLAN_INSERT_PVID;
- vsi->info.port_vlan_flags |= I40E_AQ_VSI_PVLAN_MODE_UNTAGGED;
+ vsi->info.port_vlan_flags = I40E_AQ_VSI_PVLAN_MODE_TAGGED |
+ I40E_AQ_VSI_PVLAN_INSERT_PVID |
+ I40E_AQ_VSI_PVLAN_EMOD_STR_BOTH;
ctxt.seid = vsi->seid;
memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
@@ -2040,8 +2041,9 @@ int i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
**/
void i40e_vsi_remove_pvid(struct i40e_vsi *vsi)
{
+ i40e_vlan_stripping_disable(vsi);
+
vsi->info.pvid = 0;
- i40e_vlan_rx_register(vsi->netdev, vsi->netdev->features);
}
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 685b285..2c042a26 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -388,8 +388,16 @@ static int i40e_alloc_vsi_res(struct i40e_vf *vf, enum i40e_vsi_type type)
dev_info(&pf->pdev->dev,
"LAN VSI index %d, VSI id %d\n",
vsi->idx, vsi->id);
+ /* If the port VLAN has been configured and then the
+ * VF driver was removed then the VSI port VLAN
+ * configuration was destroyed. Check if there is
+ * a port VLAN and restore the VSI configuration if
+ * needed.
+ */
+ if (vf->port_vlan_id)
+ i40e_vsi_add_pvid(vsi, vf->port_vlan_id);
f = i40e_add_filter(vsi, vf->default_lan_addr.addr,
- 0, true, false);
+ vf->port_vlan_id, true, false);
}
if (!f) {
@@ -1958,7 +1966,7 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev,
ret = i40e_vsi_add_pvid(vsi,
vlan_id | (qos << I40E_VLAN_PRIORITY_SHIFT));
else
- i40e_vlan_stripping_disable(vsi);
+ i40e_vsi_remove_pvid(vsi);
if (vlan_id) {
dev_info(&pf->pdev->dev, "Setting VLAN %d, QOS 0x%x on VF %d\n",
@@ -1978,6 +1986,10 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev,
dev_err(&pf->pdev->dev, "Unable to update VF vsi context\n");
goto error_pvid;
}
+ /* The Port VLAN needs to be saved across resets the same as the
+ * default LAN MAC address.
+ */
+ vf->port_vlan_id = le16_to_cpu(vsi->info.pvid);
ret = 0;
error_pvid:
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
index d0b712c..bf3384f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
@@ -82,6 +82,7 @@ struct i40e_vf {
struct i40e_virtchnl_ether_addr default_lan_addr;
struct i40e_virtchnl_ether_addr default_fcoe_addr;
+ u16 port_vlan_id;
/* VSI indices - actual VSI pointers are maintained in the PF structure
* When assigned, these will be non-zero, because VSI 0 is always
--
1.8.3.1
^ permalink raw reply related
* [net-next 04/15] i40e: avoid unnecessary register read
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Mitch Williams, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mitch Williams <mitch.a.williams@intel.com>
We don't need to read the base VF id. It's already stashed in the HW
struct.
Change-Id: Ib81e2f76fc40b12c966e014a856b481912cafefc
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 2c042a26..bed6022 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -600,8 +600,7 @@ static int i40e_quiesce_vf_pci(struct i40e_vf *vf)
int vf_abs_id, i;
u32 reg;
- reg = rd32(hw, I40E_PF_VT_PFALLOC);
- vf_abs_id = vf->vf_id + (reg & I40E_PF_VT_PFALLOC_FIRSTVF_MASK);
+ vf_abs_id = vf->vf_id + hw->func_caps.vf_base_id;
wr32(hw, I40E_PF_PCI_CIAA,
VF_DEVICE_STATUS | (vf_abs_id << I40E_PF_PCI_CIAA_VF_NUM_SHIFT));
--
1.8.3.1
^ permalink raw reply related
* [net-next 01/15] i40e: Record dma buffer info for dummy packets
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem
Cc: Anjali Singhai Jain, netdev, gospo, sassmann, Jesse Brandeburg,
Jeff Kirsher
In-Reply-To: <1389011436-18424-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Anjali Singhai Jain <anjali.singhai@intel.com>
Save information that we can use while cleaning the tx ring. Also record
the time_stamp since we will need it to check tx hangs.
Change-Id: Ia3f1c17f6fec9bcb7fef2542d77eac7f6c4f115c
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 2584d6b..fac4fb3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -77,7 +77,6 @@ int i40e_program_fdir_filter(struct i40e_fdir_data *fdir_data,
/* grab the next descriptor */
i = tx_ring->next_to_use;
fdir_desc = I40E_TX_FDIRDESC(tx_ring, i);
- tx_buf = &tx_ring->tx_bi[i];
tx_ring->next_to_use = (i + 1 < tx_ring->count) ? i + 1 : 0;
@@ -129,15 +128,23 @@ int i40e_program_fdir_filter(struct i40e_fdir_data *fdir_data,
/* Now program a dummy descriptor */
i = tx_ring->next_to_use;
tx_desc = I40E_TX_DESC(tx_ring, i);
+ tx_buf = &tx_ring->tx_bi[i];
tx_ring->next_to_use = (i + 1 < tx_ring->count) ? i + 1 : 0;
+ /* record length, and DMA address */
+ dma_unmap_len_set(tx_buf, len, I40E_FDIR_MAX_RAW_PACKET_LOOKUP);
+ dma_unmap_addr_set(tx_buf, dma, dma);
+
tx_desc->buffer_addr = cpu_to_le64(dma);
td_cmd = I40E_TXD_CMD | I40E_TX_DESC_CMD_DUMMY;
tx_desc->cmd_type_offset_bsz =
build_ctob(td_cmd, 0, I40E_FDIR_MAX_RAW_PACKET_LOOKUP, 0);
+ /* set the timestamp */
+ tx_buf->time_stamp = jiffies;
+
/* Force memory writes to complete before letting h/w
* know there are new descriptors to fetch. (Only
* applicable for weak-ordered memory model archs,
--
1.8.3.1
^ permalink raw reply related
* [net-next 00/15][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2014-01-06 12:30 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains updates to i40e only.
Majority of this series contains patches from Greg and Mitch to fix
up or add functionality to the PF/VF driver interactions. Notably,
a fix for SR-IOV VF port VLAN which resolved the problem of port VLAN
configurations not being persistent across VF driver loads and unloads
and enable/disable of the feature. Also do not enable the default port
on the VEB, which is designed only to bridge the PF to an Open vSwitch
or bridge. Another fix to resolve a possible memory corruption
condition where ARQ messages are written to random memory locations.
Fix a problem where the 'ip link show' command would display stale
link address information after the link address was set via the 'ip
link set' command.
Anjali provides several patches, one which saves information that can
be used while cleaning the Tx ring and useful in detecting Tx hangs.
Then provides a fixes to the admin queue shutdown function to ensure
we are shutting down the queue in the shutdown path and ensure ASQ is
alive before issuing the admin queue command.
Shannon provides a fix for get/update vsi params where the incorrect
struct was being used.
The following are changes since commit b912b2f8fc71df4c3ffa7a9fe2c2227e8bcdaa07:
net/mlx4_core: Warn if device doesn't have enough PCI bandwidth
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Anjali Singhai Jain (4):
i40e: Record dma buffer info for dummy packets
i40e: Admin queue shutdown fixes
i40e: check asq alive before notify
i40e: Do not allow AQ calls from ndo-ops
Greg Rose (4):
i40e: Fix SR-IOV VF port VLAN
i40e: Do not enable default port on the VEB
i40e: Fix VF driver MAC address configuration
i40e: Hide the Port VLAN VLAN ID
Jesse Brandeburg (1):
i40e: fix whitespace
Mitch Williams (5):
i40e: avoid unnecessary register read
i40e: use struct assign instead of memcpy
i40e: don't allocate zero size
i40e: acknowledge VFLR when disabling SR-IOV
i40e: support VFs on PFs other than 0
Shannon Nelson (1):
i40e: use correct struct for get and update vsi params
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 3 +
drivers/net/ethernet/intel/i40e/i40e_common.c | 75 ++++++++------
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 1 +
drivers/net/ethernet/intel/i40e/i40e_main.c | 45 ++++----
drivers/net/ethernet/intel/i40e/i40e_prototype.h | 5 +-
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 +-
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 114 ++++++++++++++++-----
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h | 2 +
8 files changed, 169 insertions(+), 85 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH net 1/2] macvlan: forbid L2 fowarding offload for macvtap
From: Neil Horman @ 2014-01-06 12:26 UTC (permalink / raw)
To: Jason Wang
Cc: John Fastabend, davem, netdev, linux-kernel, mst, John Fastabend
In-Reply-To: <52CA612D.2000902@redhat.com>
On Mon, Jan 06, 2014 at 03:54:21PM +0800, Jason Wang wrote:
> On 01/06/2014 03:35 PM, John Fastabend wrote:
> > On 01/05/2014 07:21 PM, Jason Wang wrote:
> >> L2 fowarding offload will bypass the rx handler of real device. This
> >> will make
> >> the packet could not be forwarded to macvtap device. Another problem
> >> is the
> >> dev_hard_start_xmit() called for macvtap does not have any
> >> synchronization.
> >>
> >> Fix this by forbidding L2 forwarding for macvtap.
> >>
> >> Cc: John Fastabend <john.r.fastabend@intel.com>
> >> Cc: Neil Horman <nhorman@tuxdriver.com>
> >> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >> ---
> >> drivers/net/macvlan.c | 5 ++++-
> >> 1 files changed, 4 insertions(+), 1 deletions(-)
> >>
> >
> > I must be missing something.
> >
> > The lower layer device should set skb->dev to the correct macvtap
> > device on receive so that in netif_receive_skb_core() the macvtap
> > handler is hit. Skipping the macvlan receive handler should be OK
> > because the switching was done by the hardware. If I read macvtap.c
> > correctly macvlan_common_newlink() is called with 'dev' where 'dev'
> > is the macvtap device. Any idea what I'm missing? I guess I'll need
> > to setup a macvtap test case.
>
> Unlike macvlan, macvtap depends on rx handler on the lower device to
> work. In this case macvlan_handle_frame() will call macvtap_receive().
> So doing netif_receive_skb_core() for macvtap device directly won't work
> since we need to forward the packet to userspace instead of kernel.
>
> For net-next.git, it may work since commit
> 6acf54f1cf0a6747bac9fea26f34cfc5a9029523 let macvtap device register an
> rx handler for itself.
I agree, this seems like it should already be fixed with the above commit. With
this the macvlan rx handler should effectively be a no-op as far as the
reception of frames is concerned. As long as the driver sets the dev correctly
to the macvtap device (and it appears to), macvtap will get frames to user
space, regardless of weather the software or hardware did the switching. If
thats the case though, I think the solution is moving that fix to -stable
(pending testing of course), rather than comming up with a new fix.
> >
> > And what synchronization are you worried about on dev_hard_start_xmit()?
> > In the L2 forwarding offload case macvlan_open() clears the NETIF_F_LLTX
> > flag so HARD_TX_LOCK protects the driver txq. We might hit this warning
> > in dev_queue_xmit() though,
> >
> > net_crit_ratelimited("Virtual device %s asks to queue packet!\n",
> >
> > Perhaps we can remove it.
>
> The problem is macvtap does not call dev_queue_xmit() for macvlan
> device. It calls macvlan_start_xmit() directly from macvtap_get_user().
> So HARD_TX_LOCK was not done for the txq.
This seems to also be fixed by 6acf54f1cf0a6747bac9fea26f34cfc5a9029523.
Macvtap does, as of that commit use dev_queue_xmit for the transmission of
frames to the lowerdevice.
Regards
Neil
^ permalink raw reply
* Re: [PATCHv5 net-next 0/8] pktgen IPsec support
From: Steffen Klassert @ 2014-01-06 12:22 UTC (permalink / raw)
To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1388719114-26244-1-git-send-email-fan.du@windriver.com>
On Fri, Jan 03, 2014 at 11:18:26AM +0800, Fan Du wrote:
> Hi, Dave
>
> Current pktgen IPsec supports only transport/ESP combinnation,
> This patchset enables user to do almost any IPsec transformation,
> both transport/tunnel mode, and AH/ESP/IPcomp type.
>
> Below configuration has been tested, and using Wireshark could decrypt
> out plain text in good formation without any checksum/auth errors:
>
> Mode/TYPE AH ESP
> Transport x x
> Tunnel x x
>
> ChangeLog
> v2:
> Rebase patchset against newest net-next.
> Patch1: Remove adding rebundant empty line spotted by Sergei.
> Patch2: Use only one dst pointing into itself to save space.
>
> v3:
> Align with David's requirement, that for user depends on orignal
> a553e4a6317b2cfc7659542c10fe43184ffe53da ("IPSEC support") from
> Jamal, their testbed configuration will not need to be changed.
>
> Add Patch2/7, Patch3/7 for statistic counting, as well as fixing
> lock usage issue.
>
> v4:
> Add Patch8/8 to document IPsec usage in pktgen, both for orignal
> implementation and this enhancement, adviced by Jamal. And comment
> format fix spoted by Sergei.
>
> v5:
> Rebase this patchset on top of xfrm locks namespace support.
>
> Fan Du (8):
> {pktgen, xfrm} Correct xfrm state lock usage when transforming
> {pktgen, xfrm} Add statistics counting when transforming
> {pktgen, xfrm} Correct xfrm_state_lock usage in xfrm_stateonly_find
> {pktgen, xfrm} Using "pgset spi xxx" to spedifiy SA for a given flow
> {pktgen, xfrm} Construct skb dst for tunnel mode transformation
> {pktgen, xfrm} Introduce xfrm_state_lookup_byspi for pktgen
> {pktgen, xfrm} Show spi value properly when ipsec turned on
> {pktgen, xfrm} Document IPsec usage in pktgen.txt
>
All applied to ipsec-next, thanks!
^ permalink raw reply
* Re: [PATCH net 2/2] net: core: explicitly select a txq before doing l2 forwarding
From: Jeff Kirsher @ 2014-01-06 12:04 UTC (permalink / raw)
To: Jason Wang, aaron.f.brown
Cc: Neil Horman, mst, e1000-devel, netdev, linux-kernel,
John Fastabend, davem
In-Reply-To: <1388978467-2075-2-git-send-email-jasowang@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 1634 bytes --]
On Mon, 2014-01-06 at 11:21 +0800, Jason Wang wrote:
> Currently, the tx queue were selected implicitly in
> ndo_dfwd_start_xmit(). The
> will cause several issues:
>
> - NETIF_F_LLTX was forced for macvlan device in this case which lead
> extra lock
> contention.
> - dev_hard_start_xmit() was called with NULL txq which bypasses the
> net device
> watchdog
> - dev_hard_start_xmit() does not check txq everywhere which will lead
> a crash
> when tso is disabled for lower device.
>
> Fix this by explicitly introducing a select queue method just for l2
> forwarding
> offload (ndo_dfwd_select_queue), and introducing dfwd_direct_xmit() to
> do the
> queue selecting and transmitting for l2 forwarding.
>
> With this fixes, NETIF_F_LLTX could be preserved for macvlan and
> there's no need
> to check txq against NULL in dev_hard_start_xmit().
>
> In the future, it was also required for macvtap l2 forwarding support
> since it
> provides a necessary synchronization method.
>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: e1000-devel@lists.sourceforge.net
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 +++++++++----
> drivers/net/macvlan.c | 3 +-
> include/linux/netdevice.h | 11 +++++++++
> net/core/dev.c | 28
> ++++++++++++++++++++++++-
> 4 files changed, 49 insertions(+), 8 deletions(-)
Thanks Jason, I have added this to my queue since it has changes against
ixgbe.
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 455 bytes --]
------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH 01/12] netfilter: avoid get_random_bytes calls
From: Florian Westphal @ 2014-01-06 11:54 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel, davem, netdev, fw
In-Reply-To: <20140105234157.GB29910@order.stressinduktion.org>
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> On Mon, Jan 06, 2014 at 12:12:55AM +0100, Pablo Neira Ayuso wrote:
> > From: Florian Westphal <fw@strlen.de>
> >
> > All these users need an initial seed value for jhash, prandom is
> > perfectly fine. This avoids draining the entropy pool where
> > its not strictly required.
>
> Secrets protecting hash tables should be rather strong.
Yes, which is why e.g. conntrack hash is not converted.
> prandom_u32() has two seeding points at boot-up. One is at late_initcall.
Yes. None of these locations are executed via initcalls, they are all
in _checkentry (i.e., run when userspace iptables inserts a rule using
the target/match), except hashlimit where its delayed until the first
address is stored (so its even later).
> Thanks to parallel boot-up this gets executed fairly early. The other one is
> when the RNG nonblocking pool is fully initialized. Only after this point we
> can assume prandom_u32() returns truely random values. In between, only
> get_random_bytes or net_get_random_once are safe for use.
Can you elaborate? If entropy estimate is really really low
(because we're booting up), why would get_random_bytes() be a better
choice [ i understand net_get_random_once() is for delaying
the actual random_bytes call until a later point in time where we've
hopefully collected more entropy ]
> To get the impression when prandom_u32 gets truely seeded, watch out
> for the message "random: nonblocking pool is initialized" in dmesg. ;)
It happens very very early on my machine, even before / is remounted
rw. I would be more interested in what happens on small embedded
boxes...
> Hmm, some of them look like good candidates for net_get_random_once. I don't
> see such a problem with draining entropy pool, especially as they don't run
> that early and they don't request so many random bits.
I specifically did not use net_get_random_once once because checkentry is
not a hotpath.
I don't see why get_random_bytes use increases the security margin, especially
considering none of these hashes have periodic run-time rehashing?
But sure, if you think this change is a problem, Pablo can just revert it.
^ permalink raw reply
* Re: [PATCH net v2 1/9] bridge: Fix the way to find old local fdb entries in br_fdb_changeaddr
From: Vlad Yasevich @ 2014-01-06 11:29 UTC (permalink / raw)
To: Toshiaki Makita
Cc: Toshiaki Makita, David S . Miller, Stephen Hemminger, netdev
In-Reply-To: <1388935578.1906.26.camel@localhost.localdomain>
On 01/05/2014 10:26 AM, Toshiaki Makita wrote:
> On Fri, 2014-01-03 at 15:46 -0500, Vlad Yasevich wrote:
>> On 01/03/2014 02:28 PM, Vlad Yasevich wrote:
>>> On 12/17/2013 07:03 AM, Toshiaki Makita wrote:
>>>> br_fdb_changeaddr() assumes that there is at most one local entry per port
>>>> per vlan. It used to be true, but since commit 36fd2b63e3b4 ("bridge: allow
>>>> creating/deleting fdb entries via netlink"), it has not been so.
>>>> Therefore, the function might fail to search a correct previous address
>>>> to be deleted and delete an arbitrary local entry if user has added local
>>>> entries manually.
>>>>
>>>> Example of problematic case:
>>>> ip link set eth0 address ee:ff:12:34:56:78
>>>> brctl addif br0 eth0
>>>> bridge fdb add 12:34:56:78:90:ab dev eth0 master
>>>> ip link set eth0 address aa:bb:cc:dd:ee:ff
>>>> Then, the address 12:34:56:78:90:ab might be deleted instead of
>>>> ee:ff:12:34:56:78, the original mac address of eth0.
>>>>
>>>> Address this issue by introducing a new flag, added_by_user, to struct
>>>> net_bridge_fdb_entry.
>>>>
>>>> Note that br_fdb_delete_by_port() has to set added_by_user to 0 in case
>>>> like:
>>>> ip link set eth0 address 12:34:56:78:90:ab
>>>> ip link set eth1 address aa:bb:cc:dd:ee:ff
>>>> brctl addif br0 eth0
>>>> bridge fdb add aa:bb:cc:dd:ee:ff dev eth0 master
>>>> brctl addif br0 eth1
>>>> brctl delif br0 eth0
>>>> In this case, kernel should delete the user-added entry aa:bb:cc:dd:ee:ff,
>>>> but it also should have been added by "brctl addif br0 eth1" originally,
>>>> so we don't delete it and treat it a new kernel-created entry.
>>>>
>>>
>>> I was looking over my patch series that adds something similar to this
>>> and noticed that you are not handing the NTF_USE case. That case was
>>> always troublesome for me as it allows for 2 different way to create
>>> the same FDB: one through br_fdb_update() and one through fdb_add_entry().
>>>
>>> It is possible, though I haven't found any users yet, that NTF_USE
>>> may be used and in that case, bridge will create a dynamic fdb and
>>> disregard all NUD flags. In case case, add_by_user will not be set
>>> either.
>>>
>>> I think that the above is broken and plan to submit a fix shortly.
>>
>> Just looked again at my NTF_USE patch and while it seems ok, the whole
>> NTF_USE usage is racy to begin with and I am really starting to question
>> it's validity.
>>
>> Presently, br_fdb_update() will not update local fdb entries. Instead
>> it will log a misleading warning... It will only let you update
>> non-local entries. This is fine for user-created entries, but any
>> operation on dynamically created entries will only persist until
>> the next packet. It also races against the packet, so there is
>> absolutely no guarantee that the values of fdb->dst and fdb->updated
>> will be consistent..
>>
>> It seems to me that the update capability of NTF_USE would actually be
>> of more value on local or user-created fdb entries.
>>
>> The fdb creation capability of NTF_USE should be disabled.
>>
>> Thoughts?
>
> I ignored NTF_USE in this patch because I regard it as emulating kernel
> creating entries after investigating git log.
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0c5c2d3089068d4aa378f7a40d2b5ad9d4f52ce8
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=292d1398983f3514a0eab13b7606df7f4730b498
>
> So I think NTF_USE shouldn't set added_by_user.
> And to emulate kernel creating entries, simply calling br_fdb_update()
> is the right way, isn't it?
You can create dynamic entries (emulating the kernel) without NTF_USE.
Just set the NUD_REACHABLE. Notice that arp cache only uses NTF_USE
to trigger and arp notification. The creation is still triggered via
other netlink flags.
The more I look at this the more I think NTF_USE should not create
an entry all by itself.
-vlad
>
> Thanks,
> Toshiaki Makita
>
^ permalink raw reply
* Re: [PATCH v2 5/5] alx: add stats to ethtool
From: Sabrina Dubroca @ 2014-01-06 10:57 UTC (permalink / raw)
To: Johannes Berg; +Cc: davem, bhutchings, netdev
In-Reply-To: <1388998990.5891.0.camel@jlt4.sipsolutions.net>
[2014-01-06, 10:03:10] Johannes Berg wrote:
> On Sat, 2014-01-04 at 17:47 +0100, Sabrina Dubroca wrote:
>
> > + __alx_update_hw_stats(hw);
> > + BUILD_BUG_ON(sizeof(hw->stats) - offsetof(struct alx_hw_stats, rx_ok) <
> > + ALX_NUM_STATS * sizeof(u64));
>
> I think you should make that != instead of <, otherwise you won't catch
> all possible differences.
With a !=, BUILD_BUG_ON is triggered if a new field is added at the
end of the structure. But adding a field doesn't break the code,
though I'm not sure allowing this is useful. The offsetof (and the
source in memcpy) also allows to add new fields at the beginning.
And the way alx_update_hw_stats is written already includes a kind of
check that all fields are present.
--
Sabrina
^ permalink raw reply
* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
From: Hannes Frederic Sowa @ 2014-01-06 10:56 UTC (permalink / raw)
To: François-Xavier Le Bail
Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki Yoshifuji, Patrick McHardy
In-Reply-To: <1389004901.29746.YahooMailBasic@web125506.mail.ne1.yahoo.com>
On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote:
> On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
>
> > On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> > > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > > index 5d42009..65c8619 100644
> > > --- a/net/ipv6/icmp.c
> > > +++ b/net/ipv6/icmp.c
> > > @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> > >
> > > saddr = &ipv6_hdr(skb)->daddr;
> > >
> > > - if (!ipv6_unicast_destination(skb))
> > > + if (!ipv6_unicast_destination(skb) &&
> > > + !(net->ipv6.anycast_src_echo_reply &&
> > > + ipv6_chk_acast_addr(net, NULL, saddr)))
> > > saddr = NULL;
>
> > I am not sure why you left out the device at ipv6_chk_acast_addr?
>
> > IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> > scope global but need to check the interface for scope link.
>
> > It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> > anycast on another interface which may not be checked here.
>
> In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent.
A counter-example would be NOARP interfaces where we also don't speak ndisc.
> Do you find some problem testing my patch ?
A quick test I did yesterday looked good.
Greetings,
Hannes
^ permalink raw reply
* [PATCH net-next] bonding: fix kstrtou8() return value verification in num_peer_notif
From: Veaceslav Falico @ 2014-01-06 10:54 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, sfeldma, Jay Vosburgh, Andy Gospodarek
It returns 0 in case of success, !0 error otherwise. Fix the improper error
verification.
Fixes: 2c9839c143bbc ("bonding: add num_grat_arp attribute netlink support")
CC: sfeldma@cumulusnetworks.com
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 9a1ea4a..011f163 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -776,7 +776,7 @@ static ssize_t bonding_store_num_peer_notif(struct device *d,
int ret;
ret = kstrtou8(buf, 10, &new_value);
- if (!ret) {
+ if (ret) {
pr_err("%s: invalid value %s specified.\n",
bond->dev->name, buf);
return ret;
--
1.8.4
^ permalink raw reply related
* Re: [PATCH 4/11] use ether_addr_equal_64bits
From: Dan Carpenter @ 2014-01-06 10:48 UTC (permalink / raw)
To: Johannes Berg
Cc: Henrique de Moraes Holschuh, Julia Lawall, kernel-janitors,
Emmanuel Grumbach, Intel Linux Wireless, John W. Linville,
linux-wireless, netdev, linux-kernel
In-Reply-To: <1388445188.18164.0.camel@jlt4.sipsolutions.net>
On Tue, Dec 31, 2013 at 12:13:08AM +0100, Johannes Berg wrote:
> On Mon, 2013-12-30 at 19:57 -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 30 Dec 2013, Johannes Berg wrote:
> > > On Mon, 2013-12-30 at 20:58 +0100, Julia Lawall wrote:
> > > > > Is there any way we could catch (sparse, or some other script?) that
> > > > > struct reorganising won't break the condition needed ("within a
> > > > > structure that contains at least two more bytes")?
> > > >
> > > > What kind of reorganizing could happen? Do you mean that the programmer
> > > > might do at some time in the future, or something the compiler might do?
> > >
> > > I'm just thinking of a programmer, e.g. changing a struct like this:
> > >
> > > struct foo {
> > > u8 addr[ETH_ALEN];
> > > - u16 dummy;
> > > };
> > >
> > > for example.
> >
> > That is easily resolved by:
> >
> > struct foo {
> > u8 addr[ETH_ALEN];
> > u16 required_padding; /* do not remove upon pain of death */
> > };
>
> That'd be a stupid waste of struct space. If anything, there should be
> *only* a comment saying that at least two bytes are needed - I'd still
> prefer an automated check.
>
This is the sort of thing where Smatch is probably the right tool. I'll
let you know how it turns out.
My guess is that it would be rare to run into this bug in real life.
Most structs have 4 or 8 byte alignment and most times the addr is not
at the end of the struct. But I also agree that this function should
only be used in a fast path.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
From: François-Xavier Le Bail @ 2014-01-06 10:41 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki Yoshifuji, Patrick McHardy
In-Reply-To: <20140105230505.GA29910@order.stressinduktion.org>
On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
> > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > index 5d42009..65c8619 100644
> > --- a/net/ipv6/icmp.c
> > +++ b/net/ipv6/icmp.c
> > @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
> >
> > saddr = &ipv6_hdr(skb)->daddr;
> >
> > - if (!ipv6_unicast_destination(skb))
> > + if (!ipv6_unicast_destination(skb) &&
> > + !(net->ipv6.anycast_src_echo_reply &&
> > + ipv6_chk_acast_addr(net, NULL, saddr)))
> > saddr = NULL;
> I am not sure why you left out the device at ipv6_chk_acast_addr?
> IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> scope global but need to check the interface for scope link.
> It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> anycast on another interface which may not be checked here.
In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent.
Do you find some problem testing my patch ?
BR
^ permalink raw reply
* Re: [PATCHv4 net-next] xfrm: Namespacify xfrm_policy_sk_bundles
From: Steffen Klassert @ 2014-01-06 10:35 UTC (permalink / raw)
To: Fan Du; +Cc: Timo Teras, Eric Dumazet, davem, netdev
In-Reply-To: <52BA9AEA.7050301@windriver.com>
On Wed, Dec 25, 2013 at 04:44:26PM +0800, Fan Du wrote:
>
>
> On 2013年12月25日 16:11, Timo Teras wrote:
> >On Wed, 25 Dec 2013 14:40:36 +0800
> >Fan Du<fan.du@windriver.com> wrote:
> >
> >>> ccing Timo
> >>>
> >>> On 2013年12月24日 18:35, Steffen Klassert wrote:
> >>>> > On Fri, Dec 20, 2013 at 11:34:41AM +0800, Fan Du wrote:
> >>>>> >>
> >>>>> >> Subject: [PATCHv4 net-next] xfrm: Namespacify
> >>>>> >> xfrm_policy_sk_bundles
> >>>>> >>
> >>>>> >> xfrm_policy_sk_bundles, protected by
> >>>>> >> net->xfrm.xfrm_policy_sk_bundle_lock should be put into netns xfrm
> >>>>> >> structure, otherwise xfrm_policy_sk_bundles can be corrupted from
> >>>>> >> different net namespace.
> >>>> >
> >>>> > I'm ok with this patch, but I wonder where we use these cached
> >>>> > socket bundles. After a quick look I see where we add and where we
> >>>> > delete them, but I can't see how we use these cached bundles.
> >>>
> >>> Interesting
> >>>
> >>> The per socket bundles is introduced by Timo in commit 80c802f3
> >>> ("xfrm: cache bundles instead of policies for outgoing flows")
> >Those existed even before. I just did systematic transformation of the
> >caching code to work on bundle level instead of policy level.
>
> Apologizes and thanks for your quick reply :)
>
> >>> But one fundamental question is why not use existing flow cache
> >>> for per socket bundles as well? then no need to create such per
> >>> sock xdst for every packet, and also share the same flow cache
> >>> flush mechanism.
> >It was needed when the flow cache cached policies. They explicitly
> >needed to check the socket for per-socket policy. So it made no sense
> >to have anything socket related in the cache.
>
> I understand your concern.
>
> per sk bundles could be distinguished by putting per sk policy pointer into
> struct flow_cache_entry, and then compare sk policy between cached policy
> against with sk policy.
Most protocols cache the used routes at the sockets, so I'm not sure if
we really need to cache them in xfrm too.
Given the fact that we don't use these cached socket policy bundles,
it would be already an improvement if we would simply remove this caching.
All we are doing here is wasting memory.
>
> And I also notice flow cache is global across different namespaces, but flow
> cache flush is doing a per-cpu(also global) operation, that's not fair for
> slim netns as compared with fat netns which floods flow cache. Maybe it's
> time to make flow cache also name space aware.
Yes, making the flow cache namespace aware would be a good thing.
^ permalink raw reply
* Re: [PATCH 4/11] use ether_addr_equal_64bits
From: Johannes Berg @ 2014-01-06 10:17 UTC (permalink / raw)
To: Julia Lawall
Cc: Ben Greear, Joe Perches, Henrique de Moraes Holschuh,
kernel-janitors, Emmanuel Grumbach, Intel Linux Wireless,
John W. Linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <alpine.DEB.2.10.1401061007160.2079@hadrien>
On Mon, 2014-01-06 at 10:09 +0100, Julia Lawall wrote:
> > BUILD_BUG_ON(sizeof(struct foo) - offsetof(struct foo, addr) < 8);
> >
> > with the user(s?) and that should catch the scenario I was worrying
> > about?
>
> OK, thanks. That is what I had in mind. But I was hoping to be able to
> put it with the structure.
Right - you might be able to do that with BUILD_BUG_ON_ZERO() as you
pointed out, I haven't looked at these macros in a while.
> Perhaps there is a way to make a macro that
> expands to a dummy function that contains the BUILD_BUG_ON? But I guess
> that would waste space?
>
> I think that 8 should be 16?
No, that should be ETH_ALEN+2 really, I guess - it's not taking into
account the size of the address member itself at all.
johannes
^ permalink raw reply
* question about ti/tlan.c
From: Julia Lawall @ 2014-01-06 10:09 UTC (permalink / raw)
To: chessman, netdev
In the function tlan_suspend, I wonder if the following code is correct?
pci_wake_from_d3(pdev, false);
I have only a moderate understanding of the code, but it seems odd that
both the suspend function and the resume function turn off wakeup events.
julia
^ permalink raw reply
* Re: [RFC PATCH net-next 0/4] net_cls for sys container
From: Libo Chen @ 2014-01-06 10:06 UTC (permalink / raw)
To: Gao feng, Cong Wang
Cc: David Miller, Jamal Hadi Salim, Li Zefan, Eric Dumazet, pshelar,
jasowang, Simon Horman, Serge Hallyn,
Linux Kernel Network Developers, cgroups, containers,
Patrick McHardy, xemul, LKML
In-Reply-To: <52CA6C80.9060002@cn.fujitsu.com>
yes
On 2014/1/6 16:42, Gao feng wrote:
> On 01/06/2014 03:54 PM, Libo Chen wrote:
>> On 2014/1/3 13:20, Cong Wang wrote:
>>> On Thu, Jan 2, 2014 at 7:11 PM, Libo Chen <clbchenlibo.chen@huawei.com> wrote:
>>>> Hi guys,
>>>>
>>>> Now, lxc created with veth can not be under control by
>>>> cls_cgroup.
>>>>
>>>> the former discussion:
>>>> http://lkml.indiana.edu/hypermail/linux/kernel/1312.1/00214.html
>>>>
>>>> In short, because cls_cgroup relys classid attached to sock
>>>> filter skb, but sock will be cleared inside dev_forward_skb()
>>>> in veth_xmit().
>>>
>>>
>>> So what are you trying to achieve here?
>>
>> sys container using veth can be controlled by cls_cgroup basing on physic network interface
>>
>
> It's a problem about virtual nic, not container/net namespace.
yes
>
> If veth device is running in host. the skb is transmitted firstly by veth device and then delivered
> by physical device. if you set both qdisc rule on veth and physical device. which qdisc rule will take
> effect?
both, the end result depends on a smaller.
>
> In your patch, both qdisc rule are effective. it looks strange.
>
qdisc is based nic, does not distinguish virtual or physics. if you are all set,
it means that what you want. so the logic is not the problemI and this appears to be normal.
thanks,
Libo
> .
>
^ permalink raw reply
* Re: [PATCH 4/11] use ether_addr_equal_64bits
From: Julia Lawall @ 2014-01-06 9:35 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Emmanuel Grumbach, Johannes Berg, Henrique de Moraes Holschuh,
Julia Lawall, kernel-janitors@vger.kernel.org, Emmanuel Grumbach,
Intel Linux Wireless, John W. Linville, linux-wireless,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdXZQO7jj+V2mMyyLgDoz6iSLJ25+55Yow-Acb1=tE5sMg@mail.gmail.com>
On Mon, 6 Jan 2014, Geert Uytterhoeven wrote:
> On Tue, Dec 31, 2013 at 7:26 AM, Emmanuel Grumbach <egrumbach@gmail.com> wrote:
> > On Tue, Dec 31, 2013 at 1:13 AM, Johannes Berg
> > <johannes@sipsolutions.net> wrote:
> >>
> >> On Mon, 2013-12-30 at 19:57 -0200, Henrique de Moraes Holschuh wrote:
> >> > On Mon, 30 Dec 2013, Johannes Berg wrote:
> >> > > On Mon, 2013-12-30 at 20:58 +0100, Julia Lawall wrote:
> >> > > > > Is there any way we could catch (sparse, or some other script?) that
> >> > > > > struct reorganising won't break the condition needed ("within a
> >> > > > > structure that contains at least two more bytes")?
> >> > > >
> >> > > > What kind of reorganizing could happen? Do you mean that the programmer
> >> > > > might do at some time in the future, or something the compiler might do?
> >> > >
> >> > > I'm just thinking of a programmer, e.g. changing a struct like this:
> >> > >
> >> > > struct foo {
> >> > > u8 addr[ETH_ALEN];
> >> > > - u16 dummy;
> >> > > };
> >> > >
> >> > > for example.
> >> >
> >> > That is easily resolved by:
> >> >
> >> > struct foo {
> >> > u8 addr[ETH_ALEN];
> >> > u16 required_padding; /* do not remove upon pain of death */
> >> > };
>
> Adding the u16 also changes the alignment of the whole struct. So it may
> cost one additional byte _in front of_ the struct.
>
> <asking-stupid-question-see-answer-below>
> While you're at it, why not just making a new 64-bit aligned type for
> Ethernet addresses, so it'll also work for
> !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?
> </asking-stupid-question-see-answer-below>
>
> >> That'd be a stupid waste of struct space. If anything, there should be
> >> *only* a comment saying that at least two bytes are needed - I'd still
> >> prefer an automated check.
> >
> > Frankly I am not sure I like the patch. This flow is not a fast path
>
> I also don't like it. To me this sounds like wasting space for nothing.
> BTW, would it be that more expensive to always do a 32+16 bit
> comparison?
No space is wasted by the patch. The patch is only generated in the case
where there is currently enough space.
julia
> > at all. While I don't really care for the waste in iwlwifi (because
> > there isn't), I don't see the real point is make the code more
> > sensitive to changes to earn basically nothing.
>
> Thanks to this discussion, my eye fell on:
>
> static inline unsigned compare_ether_addr(const u8 *addr1, const u8 *addr2)
> {
> const u16 *a = (const u16 *) addr1;
> const u16 *b = (const u16 *) addr2;
>
> BUILD_BUG_ON(ETH_ALEN != 6);
> return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) != 0;
> }
>
> What if addr1 or addr2 are odd, and this is running on an architecture that
> doesn't support unaligned accesses at all?? Have we been lucky forever?
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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: [PATCH 4/11] use ether_addr_equal_64bits
From: Geert Uytterhoeven @ 2014-01-06 9:24 UTC (permalink / raw)
To: Emmanuel Grumbach
Cc: Johannes Berg, Henrique de Moraes Holschuh, Julia Lawall,
kernel-janitors@vger.kernel.org, Emmanuel Grumbach,
Intel Linux Wireless, John W. Linville, linux-wireless,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CANUX_P0PX1g8uHJUHYoB4UyMMgbtLas0_pf4TYrLTgND5V37rg@mail.gmail.com>
On Tue, Dec 31, 2013 at 7:26 AM, Emmanuel Grumbach <egrumbach@gmail.com> wrote:
> On Tue, Dec 31, 2013 at 1:13 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>>
>> On Mon, 2013-12-30 at 19:57 -0200, Henrique de Moraes Holschuh wrote:
>> > On Mon, 30 Dec 2013, Johannes Berg wrote:
>> > > On Mon, 2013-12-30 at 20:58 +0100, Julia Lawall wrote:
>> > > > > Is there any way we could catch (sparse, or some other script?) that
>> > > > > struct reorganising won't break the condition needed ("within a
>> > > > > structure that contains at least two more bytes")?
>> > > >
>> > > > What kind of reorganizing could happen? Do you mean that the programmer
>> > > > might do at some time in the future, or something the compiler might do?
>> > >
>> > > I'm just thinking of a programmer, e.g. changing a struct like this:
>> > >
>> > > struct foo {
>> > > u8 addr[ETH_ALEN];
>> > > - u16 dummy;
>> > > };
>> > >
>> > > for example.
>> >
>> > That is easily resolved by:
>> >
>> > struct foo {
>> > u8 addr[ETH_ALEN];
>> > u16 required_padding; /* do not remove upon pain of death */
>> > };
Adding the u16 also changes the alignment of the whole struct. So it may
cost one additional byte _in front of_ the struct.
<asking-stupid-question-see-answer-below>
While you're at it, why not just making a new 64-bit aligned type for
Ethernet addresses, so it'll also work for
!CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?
</asking-stupid-question-see-answer-below>
>> That'd be a stupid waste of struct space. If anything, there should be
>> *only* a comment saying that at least two bytes are needed - I'd still
>> prefer an automated check.
>
> Frankly I am not sure I like the patch. This flow is not a fast path
I also don't like it. To me this sounds like wasting space for nothing.
BTW, would it be that more expensive to always do a 32+16 bit
comparison?
> at all. While I don't really care for the waste in iwlwifi (because
> there isn't), I don't see the real point is make the code more
> sensitive to changes to earn basically nothing.
Thanks to this discussion, my eye fell on:
static inline unsigned compare_ether_addr(const u8 *addr1, const u8 *addr2)
{
const u16 *a = (const u16 *) addr1;
const u16 *b = (const u16 *) addr2;
BUILD_BUG_ON(ETH_ALEN != 6);
return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) != 0;
}
What if addr1 or addr2 are odd, and this is running on an architecture that
doesn't support unaligned accesses at all?? Have we been lucky forever?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH net-next v2 6/6] r8152: support RTL8153
From: Bjørn Mork @ 2014-01-06 9:21 UTC (permalink / raw)
To: hayeswang; +Cc: oliver, netdev, 'nic_swsd', linux-kernel, linux-usb
In-Reply-To: <AA7EB9EA13B74B14BBE08C4278F7948C@realtek.com.tw>
hayeswang <hayeswang@realtek.com> writes:
> Bjørn Mork [mailto:bjorn@mork.no]
> [...]
>> Sorry, but then this makes even less sense. The active USB
>> configuration is user selectable and you should make any of
>> them work if
>> possible. Why can't the drivers figure out this at runtime?
>
> Excuse me. I have no idea about how to switch the configuration at
> the runtime ,and how to change the default configuration when a
> USB device is plugged. When a user select one of the configurations,
> he/she would wish to fix the configuration number after rebooting or
> after the dangle is unplugged and plugged again. For these reasons,
> this is the simple way which I could think. Maybe I choose the wrong
> method because I don't know how to satisfy the requests. May you
> provide me the relative information? Then, I could replace the current
> method. Thanks.
I apologise if I misunderstood you. When you mentioned multiple
configurations, then I assumed it was a device similar to this one
(partial output from /sys/kernel/debug/usb/devices):
T: Bus=03 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#= 4 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 2
P: Vendor=1199 ProdID=68a2 Rev= 0.06
S: Manufacturer=Sierra Wireless, Incorporated
S: Product=MC7710
S: SerialNumber=358178040092316
C: #Ifs= 6 Cfg#= 1 Atr=e0 MxPwr= 0mA
I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=
E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=
E: Ad=83(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I: If#= 8 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=
E: Ad=85(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I: If#=19 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=
E: Ad=87(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=88(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I: If#=20 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=
E: Ad=89(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=8a(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
C:* #Ifs= 2 Cfg#= 2 Atr=e0 MxPwr= 0mA
A: FirstIf#=12 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
I:* If#=12 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
I: If#=13 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
I:* If#=13 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
Here you can see 2 configurations, where cfg #2 is active using a class
driver. This is what Linux will choose in this case because the first
interface of cfg #1 is vendor specific. But an end user can change it
by writing to the bConfigurationValue sysfs attribute:
nemi:/tmp# echo 1 > /sys/bus/usb/devices/3-4/bConfigurationValue
giving this result:
T: Bus=03 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#= 4 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 2
P: Vendor=1199 ProdID=68a2 Rev= 0.06
S: Manufacturer=Sierra Wireless, Incorporated
S: Product=MC7710
S: SerialNumber=358178040092316
C:* #Ifs= 6 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial
E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I:* If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial
E: Ad=83(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I:* If#= 8 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
E: Ad=85(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I:* If#=19 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
E: Ad=87(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=88(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I:* If#=20 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
E: Ad=89(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=8a(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
C: #Ifs= 2 Cfg#= 2 Atr=e0 MxPwr= 0mA
A: FirstIf#=12 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
I: If#=12 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=
E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
I: If#=13 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=
I: If#=13 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
Exactly the same device, but now cfg #1 is active and a different set of
drivers have bound to the interfaces. This is possible because none of
the involved drivers disable the support for this device at build-time.
Instead they use the available interface descriptors for matching and
probing supported functions.
End users will of course normally not go around writing stuff to sysfs
attributes like this. Creating an udev rule to select a specific
counfiguration when the device is plugged is more useful for normal
usage.
My point was that this sort of runtime configuration change should be
possible, assuming my understanding of the device layout is correct.
Which it isn't necessarily. I don't have any such device and I haven't
seen any USB descriptor dump from one either.
Bjørn
^ permalink raw reply
* Re: [PATCH 4/11] use ether_addr_equal_64bits
From: Julia Lawall @ 2014-01-06 9:20 UTC (permalink / raw)
To: Johannes Berg
Cc: Julia Lawall, Joe Perches, Henrique de Moraes Holschuh,
kernel-janitors, Emmanuel Grumbach, Intel Linux Wireless,
John W. Linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1388999220.5891.3.camel@jlt4.sipsolutions.net>
On Mon, 6 Jan 2014, Johannes Berg wrote:
> On Mon, 2014-01-06 at 10:04 +0100, Julia Lawall wrote:
>
> > OK, the question was expressed badly. Is there any way to use the value
> > to trigger an action at build time? The only way I kow to trigger an
> > action is with #error, but #error is processed by cpp, which doesn't know
> > about the size of types.
>
> That's exactly BUILD_BUG_ON(), I believe.
Maybe BUILD_BUG_ON_ZERO? It's goal seems to be to return an integer,
which could then be used as an array size in a dummy structure. The
structure declaration would not generate any code. The structure would
have to have a name, thoough.
julia
^ permalink raw reply
* [PATCH net] r8152: add terminating newline
From: Hayes Wang @ 2014-01-06 9:19 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Hayes Wang
The netif_err() and netif_info() should have the terminating newline
at the end of the message.
Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
---
drivers/net/usb/r8152.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5107372..3d8a565 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2135,11 +2135,11 @@ static int rtl8152_probe(struct usb_interface *intf,
ret = register_netdev(netdev);
if (ret != 0) {
- netif_err(tp, probe, netdev, "couldn't register the device");
+ netif_err(tp, probe, netdev, "couldn't register the device\n");
goto out1;
}
- netif_info(tp, probe, netdev, "%s", DRIVER_VERSION);
+ netif_info(tp, probe, netdev, "%s\n", DRIVER_VERSION);
return 0;
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [patch iproute2 v3 2/2] add support for IFA_F_MANAGETEMPADDR
From: Jiri Pirko @ 2014-01-06 9:17 UTC (permalink / raw)
To: netdev; +Cc: stephen, thaller
In-Reply-To: <1388999830-1753-1-git-send-email-jiri@resnulli.us>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
include/linux/if_addr.h | 1 +
ip/ipaddress.c | 11 ++++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/linux/if_addr.h b/include/linux/if_addr.h
index cced59f..e1e95ce 100644
--- a/include/linux/if_addr.h
+++ b/include/linux/if_addr.h
@@ -45,6 +45,7 @@ enum {
#define IFA_F_DEPRECATED 0x20
#define IFA_F_TENTATIVE 0x40
#define IFA_F_PERMANENT 0x80
+#define IFA_F_MANAGETEMPADDR 0x0100
struct ifa_cacheinfo {
__u32 ifa_prefered;
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1e3f22c..b0d54fe 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -82,7 +82,7 @@ static void usage(void)
fprintf(stderr, " tentative | deprecated | dadfailed | temporary |\n");
fprintf(stderr, " CONFFLAG-LIST ]\n");
fprintf(stderr, "CONFFLAG-LIST := [ CONFFLAG-LIST ] CONFFLAG\n");
- fprintf(stderr, "CONFFLAG := [ home | nodad ]\n");
+ fprintf(stderr, "CONFFLAG := [ home | nodad | mngtmpaddr ]\n");
fprintf(stderr, "LIFETIME := [ valid_lft LFT ] [ preferred_lft LFT ]\n");
fprintf(stderr, "LFT := forever | SECONDS\n");
@@ -703,6 +703,10 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
ifa_flags &= ~IFA_F_NODAD;
fprintf(fp, "nodad ");
}
+ if (ifa_flags & IFA_F_MANAGETEMPADDR) {
+ ifa_flags &= ~IFA_F_MANAGETEMPADDR;
+ fprintf(fp, "mngtmpaddr ");
+ }
if (!(ifa_flags & IFA_F_PERMANENT)) {
fprintf(fp, "dynamic ");
} else
@@ -1126,6 +1130,9 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
} else if (strcmp(*argv, "nodad") == 0) {
filter.flags |= IFA_F_NODAD;
filter.flagmask |= IFA_F_NODAD;
+ } else if (strcmp(*argv, "mngtmpaddr") == 0) {
+ filter.flags |= IFA_F_MANAGETEMPADDR;
+ filter.flagmask |= IFA_F_MANAGETEMPADDR;
} else if (strcmp(*argv, "dadfailed") == 0) {
filter.flags |= IFA_F_DADFAILED;
filter.flagmask |= IFA_F_DADFAILED;
@@ -1345,6 +1352,8 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
ifa_flags |= IFA_F_HOMEADDRESS;
} else if (strcmp(*argv, "nodad") == 0) {
ifa_flags |= IFA_F_NODAD;
+ } else if (strcmp(*argv, "mngtmpaddr") == 0) {
+ ifa_flags |= IFA_F_MANAGETEMPADDR;
} else {
if (strcmp(*argv, "local") == 0) {
NEXT_ARG();
--
1.8.3.1
^ permalink raw reply related
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