* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Sekhar Nori @ 2013-08-23 17:09 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52179448.1020906@ti.com>
On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
>> On 23.08.2013 16:23, Santosh Shilimkar wrote:
>>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>>
>>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>>> + {
>>>> + .compatible = "ti,am3352-cpsw",
>>>
>>> I didn't notice this earlier, but can't you use the IP version
>>> as a compatible instead of using a SOC name. Whats really SOC specific
>>> on this IP ? Sorry i have missed any earlier discussion on this but
>>> this approach doesn't seem good. Its like adding SOC checks in the
>>> driver subsystem.
>>
>> As I already mentioned in the cover letter and in the commit message, I
>> just don't know which criteria makes most sense here.
>>
>> On a general note, I would say that chances that this exactly IP core
>> with the same version number will appear on some other silicon which
>> doesn't support the control mode register in an AM33xx fashion, is not
>> necessarily negligible.
>>
>> So what that new compatible string denotes is the cpsw in a version as
>> found on am3352 SoCs, which is actually exactly what it does.
>>
>> I don't have a strong opinion here, but see your point. I just don't
>> have a better idea on how to treat that.
>>
> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
If this could be handled using IP version then the right way would be to
just read the IP version from hardware and use it. No need of DT property.
Thanks,
Sekhar
^ permalink raw reply
* Re: [net-next v2 3/8] i40e: driver ethtool core
From: Stefan Assmann @ 2013-08-23 17:08 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, Jesse Brandeburg, netdev, gospo, Shannon Nelson,
PJ Waskiewicz, e1000-devel
In-Reply-To: <1377224142-25160-4-git-send-email-jeffrey.t.kirsher@intel.com>
On 23.08.2013 04:15, Jeff Kirsher wrote:
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> This patch contains the ethtool interface and implementation.
>
> The goal in this patch series is minimal functionality while not
> including much in the way of "set support."
[...]
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
[...]
> +#define I40E_QUEUE_STATS_LEN(n) \
> + ((((struct i40e_netdev_priv *)netdev_priv((n)))->vsi->num_queue_pairs + \
> + ((struct i40e_netdev_priv *)netdev_priv((n)))->vsi->num_queue_pairs) * 2)
> +#define I40E_GLOBAL_STATS_LEN ARRAY_SIZE(i40e_gstrings_stats)
> +#define I40E_NETDEV_STATS_LEN ARRAY_SIZE(i40e_gstrings_net_stats)
> +#define I40E_VSI_STATS_LEN(n) (I40E_NETDEV_STATS_LEN + \
Please use tabs for spacing here.
> + I40E_QUEUE_STATS_LEN((n)))
> +#define I40E_PFC_STATS_LEN ( \
> + (FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_rx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_rx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_tx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_tx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_2_xoff)) \
> + / sizeof(u64))
> +#define I40E_PF_STATS_LEN(n) (I40E_GLOBAL_STATS_LEN + \
Here as well.
[...]
> +static void i40e_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
> + void *p)
> +{
> + struct i40e_netdev_priv *np = netdev_priv(netdev);
> + struct i40e_pf *pf = np->vsi->back;
> + struct i40e_hw *hw = &pf->hw;
> + u32 *reg_buf = p;
> + int i, j, ri;
> + u32 reg;
> +
> + /* Tell ethtool which driver-version-specific regs output we have.
> + *
> + * At some point, if we have ethtool doing special formatting of
> + * this data, it will rely on this version number to know how to
> + * interpret things. Hence, this needs to be updated if/when the
> + * diags register table is changed.
> + */
> + regs->version = 1;
> +
> + /* loop through the diags reg table for what to print */
> + ri = 0;
> + for (i = 0; i40e_reg_list[i].offset != 0; i++) {
> + for (j = 0; j < i40e_reg_list[i].elements; j++) {
> + reg = i40e_reg_list[i].offset
> + + (j * i40e_reg_list[i].stride);
> + reg_buf[ri++] = rd32(hw, reg);
> + }
> + }
> +
> + return;
void function, no return necessary.
> +}
[...]
> +static void i40e_get_ethtool_stats(struct net_device *netdev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct i40e_netdev_priv *np = netdev_priv(netdev);
> + struct i40e_vsi *vsi = np->vsi;
> + struct i40e_pf *pf = vsi->back;
> + struct rtnl_link_stats64 *net_stats = i40e_get_vsi_stats_struct(vsi);
> + char *p;
> + int i, j;
> +
> + i40e_update_stats(vsi);
> +
> + i = 0;
This could be avoided by int i = 0 few lines above.
> + for (j = 0; j < I40E_NETDEV_STATS_LEN; j++) {
> + p = (char *)net_stats + i40e_gstrings_net_stats[j].stat_offset;
> + data[i++] = (i40e_gstrings_net_stats[j].sizeof_stat ==
> + sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
> + }
> + for (j = 0; j < vsi->num_queue_pairs; j++) {
> + data[i++] = vsi->tx_rings[j].tx_stats.packets;
> + data[i++] = vsi->tx_rings[j].tx_stats.bytes;
> + }
> + for (j = 0; j < vsi->num_queue_pairs; j++) {
> + data[i++] = vsi->rx_rings[j].rx_stats.packets;
> + data[i++] = vsi->rx_rings[j].rx_stats.bytes;
> + }
> + if (vsi == pf->vsi[pf->lan_vsi]) {
> + for (j = 0; j < I40E_GLOBAL_STATS_LEN; j++) {
> + p = (char *)pf + i40e_gstrings_stats[j].stat_offset;
> + data[i++] = (i40e_gstrings_stats[j].sizeof_stat ==
> + sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++) {
> + data[i++] = pf->stats.priority_xon_tx[j];
> + data[i++] = pf->stats.priority_xoff_tx[j];
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++) {
> + data[i++] = pf->stats.priority_xon_rx[j];
> + data[i++] = pf->stats.priority_xoff_rx[j];
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++)
> + data[i++] = pf->stats.priority_xon_2_xoff[j];
> + }
> +
> + return;
Another void function.
[...]
> +static struct ethtool_ops i40e_ethtool_ops = {
> + .get_settings = i40e_get_settings,
> + .get_drvinfo = i40e_get_drvinfo,
> + .get_regs_len = i40e_get_regs_len,
> + .get_regs = i40e_get_regs,
> + .nway_reset = i40e_nway_reset,
> + .get_link = ethtool_op_get_link,
> + .get_wol = i40e_get_wol,
> + .get_ringparam = i40e_get_ringparam,
> + .set_ringparam = i40e_set_ringparam,
> + .get_pauseparam = i40e_get_pauseparam,
> + .get_msglevel = i40e_get_msglevel,
> + .set_msglevel = i40e_set_msglevel,
> + .get_rxnfc = i40e_get_rxnfc,
> + .set_rxnfc = i40e_set_rxnfc,
> + .self_test = i40e_diag_test,
> + .get_strings = i40e_get_strings,
> + .set_phys_id = i40e_set_phys_id,
> + .get_sset_count = i40e_get_sset_count,
> + .get_ethtool_stats = i40e_get_ethtool_stats,
> + .get_coalesce = i40e_get_coalesce,
> + .set_coalesce = i40e_set_coalesce,
> + .get_ts_info = i40e_get_ts_info,
> +};
It would be nice if you could use tabs for spacing here.
Stefan
^ permalink raw reply
* Re: [PATCH 2/2] genl: Hold reference on correct module while netlink-dump.
From: Pravin Shelar @ 2013-08-23 17:05 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev@vger.kernel.org, Jesse Gross
In-Reply-To: <1377242625.14021.5.camel@jlt4.sipsolutions.net>
On Fri, Aug 23, 2013 at 12:23 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> +extern int __genl_register_family(struct genl_family *family, struct module *module);
>
> Why add an extra argument instead of just assigning family->module in
> the inline wrapper(s):
>
> static inline int genl_register_family(struct genl_family *family)
> {
> family->module = THIS_MODULE;
> return __genl_register_family(family);
> }
>
This is better, I will send updated patch.
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 17:05 UTC (permalink / raw)
To: Mugunthan V N
Cc: Daniel Mack, netdev, bcousson, nsekhar, sergei.shtylyov, davem,
ujhelyi.m, vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <521791C1.2060803@ti.com>
On Friday 23 August 2013 12:45 PM, Mugunthan V N wrote:
> On Friday 23 August 2013 07:53 PM, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>>> In order to support features that are specific to the AM335x IP, we have
>>> to add hardware types and another compatible string.
>>>
>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>> ---
>>> Documentation/devicetree/bindings/net/cpsw.txt | 3 ++-
>>> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++------
>>> drivers/net/ethernet/ti/cpsw.h | 1 +
>>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>>> index 4e5ca54..b717458 100644
>>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>>> @@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>>> ------------------------------------------------------
>>>
>>> Required properties:
>>> -- compatible : Should be "ti,cpsw"
>>> +- compatible : Should be "ti,cpsw" for generic cpsw support, or
>>> + "ti,am3352-cpsw" for AM3352 SoCs
>>> - reg : physical base address and size of the cpsw
>>> registers map.
>>> An optional third memory region can be supplied if
>>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>>> index 7a25ff4..73c44cb6 100644
>>> --- a/drivers/net/ethernet/ti/cpsw.c
>>> +++ b/drivers/net/ethernet/ti/cpsw.c
>>> @@ -155,6 +155,11 @@ do { \
>>> ((priv->data.dual_emac) ? priv->emac_port : \
>>> priv->data.active_slave)
>>>
>>> +enum {
>>> + CPSW_TYPE_GENERIC,
>>> + CPSW_TYPE_AM33XX
>>> +};
>>> +
>>> static int debug_level;
>>> module_param(debug_level, int, 0);
>>> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
>>> @@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
>>> slave->port_vlan = data->dual_emac_res_vlan;
>>> }
>>>
>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>> + {
>>> + .compatible = "ti,am3352-cpsw",
>> I didn't notice this earlier, but can't you use the IP version
>> as a compatible instead of using a SOC name. Whats really SOC specific
>> on this IP ? Sorry i have missed any earlier discussion on this but
>> this approach doesn't seem good. Its like adding SOC checks in the
>> driver subsystem.
>>
>>
> But the same IP can be used in different SoC as well where the control
> register may be different as per the Silicon Integration team's decision?
>
> Ideally there should be a separate control module driver so that it can
> take care of different SoC related needs.
>
Instead of having one control module driver to address all the requirements
in various IP's, you could specifically address that in subsystem driver layer.
SOC guys invariably use control modules for certain functionality since it
is easy and quick fix. And their is uniformity in the way they are
implemented for different IP needs.
In this specific case, if the CPSW IP version has some part of the
functionality implemented using control module that is just harfaware
choice. Its not mandatory to partition that in two sub-system. Rather
consider it as a special IP version and treat it accordingly in the
driver.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH 1/2] genl: Fix genl dumpit() locking.
From: Pravin Shelar @ 2013-08-23 17:05 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev@vger.kernel.org, Jesse Gross
In-Reply-To: <1377242432.14021.2.camel@jlt4.sipsolutions.net>
On Fri, Aug 23, 2013 at 12:20 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> @@ -572,15 +594,29 @@ static int genl_family_rcv_msg(struct
>> genl_family *family,
>> return -EPERM;
>>
>> if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
>> - struct netlink_dump_control c = {
>> - .dump = ops->dumpit,
>> - .done = ops->done,
>> - };
>> + struct netlink_dump_control c;
>> + int rc;
>>
>> if (ops->dumpit == NULL)
>> return -EOPNOTSUPP;
>>
>> - return netlink_dump_start(net->genl_sock, skb, nlh,
>> &c);
>> + memset(&c, 0, sizeof(c));
>> + if (!family->parallel_ops) {
>> + genl_unlock();
>> + c.data = ops;
>> + c.dump = genl_lock_dumpit;
>> + if (ops->done)
>> + c.done = genl_lock_done;
>> + } else {
>> + c.dump = ops->dumpit;
>> + c.done = ops->done;
>> + }
>> +
>> + rc = netlink_dump_start(net->genl_sock, skb, nlh, &c);
>> + if (!family->parallel_ops)
>> + genl_lock();
>> + return rc;
>> +
>
> I think this piece would be easier to read if you call
> netlink_dump_start() separately in the two branches. If you also move
> the "c" variable into the branches then you can keep initializing it
> with an explicit initializer which would also be more readable IMHO.
>
ok, I will update patch.
> Either way, this seems fine. I still think that not assigning cb_mutex
> wasn't the smartest idea, but I'll reply over in the other thread.
^ permalink raw reply
* RE: [net-next v2 1/8] i40e: main driver core
From: Nelson, Shannon @ 2013-08-23 17:00 UTC (permalink / raw)
To: David Miller, Kirsher, Jeffrey T
Cc: Brandeburg, Jesse, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com, Waskiewicz Jr, Peter P,
e1000-devel@lists.sourceforge.net
In-Reply-To: <20130823.002805.1975660490102085087.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 23, 2013 12:28 AM
> To: Kirsher, Jeffrey T
> Cc: Brandeburg, Jesse; netdev@vger.kernel.org; gospo@redhat.com;
> sassmann@redhat.com; Nelson, Shannon; Waskiewicz Jr, Peter P; e1000-
> devel@lists.sourceforge.net
> Subject: Re: [net-next v2 1/8] i40e: main driver core
Thanks, Dave, for your time and the notes.
>
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Thu, 22 Aug 2013 19:15:35 -0700
>
> > +enum i40e_status_code i40e_allocate_dma_mem_d(struct i40e_hw *hw,
> > + struct i40e_dma_mem *mem,
> > + u64 size, u32 alignment)
> > +{
> ...
> > + mem->va = dma_alloc_coherent(&pf->pdev->dev, mem->size,
> > + &mem->pa, GFP_ATOMIC | __GFP_ZERO);
>
> First, I see no reason to specify GFP_ATOMIC here, code paths that
> call this thing even have comments above them like:
>
> --------------------
> + * Do *NOT* hold the lock when calling this as the memory allocation
> routines
> + * called are not going to be atomic context safe
> --------------------
>
> Secondly, use dma_zalloc_coherent() if you want __GFP_ZERO.
Sure, we'll adjust.
>
> > +static int i40e_get_lump(struct i40e_lump_tracking *pile, u16 needed,
> u16 id)
> > +{
> > + int i = 0, j = 0;
> > + int ret = I40E_ERR_NO_MEMORY;
> > +
> > + if (pile == NULL || needed == 0 || id >= I40E_PILE_VALID_BIT) {
> > + pr_info("%s: param err: pile=%p needed=%d id=0x%04x\n",
> > + __func__, pile, needed, id);
> > + return I40E_ERR_PARAM;
>
> Since there is absolutely no context passed into these helper routines,
> the log messages are less useful than they could be. If you did this
> right you could use netdev_info() or dev_info() here.
Perhaps we went a little too far in trying for loosely coupled code? We'll add enough context for dev_info().
>
> > +void i40e_pf_reset_stats(struct i40e_pf *pf)
> > +{
> > + memset(&pf->stats, 0, sizeof(struct i40e_hw_port_stats));
> > + memset(&pf->stats_offsets, 0, sizeof(struct i40e_hw_port_stats));
> > + pf->stat_offsets_loaded = false;
> > +
> > +}
>
> Spurious empty line at end of that function.
Obviously we need another pass at catching these little whitespace issues.
>
> > + flush(hw);
>
> I think this brief and common name is asking for namespace collision
> problems. Maybe name it i40e_flush or i40e_hw_flush or something like
> that.
Good call - thanks.
>
> > +{
> > + int i;
> > + struct i40e_pf *pf = vsi->back;
>
> Please order local variable declarations from longest line to shortest.
Will do, on these and the rest.
[...]
>
> > +static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
> > +{
> > + int num_tc = 0, i;
> > + /* Scan the ETS Config Priority Table to find
> > + * traffic class enabled for a given priority
> > + * and use the traffic class index to get the
> > + * number of traffic classes enabled
> > + */
>
> Please put an empty line between the local variables and
> the rest of the function.
Yep.
[...]
>
> > +static inline int i40e_prev_power_of_2(int n)
> > +{
> > + int p = n;
> > + --p;
> > + p |= p >> 1;
> > + p |= p >> 2;
> > + p |= p >> 4;
> > + p |= p >> 8;
> > + p |= p >> 16;
> > + if (p == (n - 1))
> > + return n; /* it was already a power of 2 */
> > + p >>= 1;
> > + return ++p;
> > +}
>
> I think something using rounddown_pow_of_two() would accomplish this.
>
> Perhaps:
>
> if (!is_power_of_2(x))
> x = rounddown_pow_of_two(x);
Oh, didn't know about that one, thanks!
sln
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 16:56 UTC (permalink / raw)
To: Daniel Mack
Cc: mugunthanvnm, sergei.shtylyov, d-gerlach, netdev, nsekhar,
vaibhav.bedia, devicetree, bcousson, ujhelyi.m, linux-omap, davem,
linux-arm-kernel
In-Reply-To: <52178E20.5040404@gmail.com>
On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
> On 23.08.2013 16:23, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>
>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>> + {
>>> + .compatible = "ti,am3352-cpsw",
>>
>> I didn't notice this earlier, but can't you use the IP version
>> as a compatible instead of using a SOC name. Whats really SOC specific
>> on this IP ? Sorry i have missed any earlier discussion on this but
>> this approach doesn't seem good. Its like adding SOC checks in the
>> driver subsystem.
>
> As I already mentioned in the cover letter and in the commit message, I
> just don't know which criteria makes most sense here.
>
> On a general note, I would say that chances that this exactly IP core
> with the same version number will appear on some other silicon which
> doesn't support the control mode register in an AM33xx fashion, is not
> necessarily negligible.
>
> So what that new compatible string denotes is the cpsw in a version as
> found on am3352 SoCs, which is actually exactly what it does.
>
> I don't have a strong opinion here, but see your point. I just don't
> have a better idea on how to treat that.
>
So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
That way if in future if someone uses those features, they can use
this compatible if they don't they use the one which suites that
SOC.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH v4 4/5] net: ethernet: cpsw: add support for hardware interface mode config
From: Mugunthan V N @ 2013-08-23 16:50 UTC (permalink / raw)
To: Daniel Mack
Cc: netdev, bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <1377267365-24057-5-git-send-email-zonque@gmail.com>
On Friday 23 August 2013 07:46 PM, Daniel Mack wrote:
> The cpsw currently lacks code to properly set up the hardware interface
> mode on AM33xx. Other platforms might be equally affected.
>
> Usually, the bootloader will configure the control module register, so
> probably that's why such support wasn't needed in the past. In suspend
> mode though, this register is modified, and so it needs reprogramming
> after resume.
>
> This patch adds code that makes use of the previously added and optional
> support for passing the control mode register, and configures the
> correct register bits when the slave is opened.
>
> The AM33xx also has a bit for each slave to configure the RMII reference
> clock direction. Setting it is now supported by a per-slave DT property.
>
> This code path introducted by this patch is currently exclusive for
> am33xx.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
> Documentation/devicetree/bindings/net/cpsw.txt | 2 +
> drivers/net/ethernet/ti/cpsw.c | 61 ++++++++++++++++++++++++++
> drivers/net/ethernet/ti/cpsw.h | 1 +
> 3 files changed, 64 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index b717458..0895a51 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -34,6 +34,8 @@ Required properties:
> - phy_id : Specifies slave phy id
> - phy-mode : The interface between the SoC and the PHY (a string
> that of_get_phy_mode() can understand)
> +- ti,rmii-clock-ext : If present, the driver will configure the RMII
> + interface to external clock usage
> - mac-address : Specifies slave MAC address
>
> Optional properties:
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 73c44cb6..86b8f7a 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -138,6 +138,13 @@ do { \
> #define CPSW_CMINTMAX_INTVL (1000 / CPSW_CMINTMIN_CNT)
> #define CPSW_CMINTMIN_INTVL ((1000 / CPSW_CMINTMAX_CNT) + 1)
>
> +#define AM33XX_GMII_SEL_MODE_MII (0)
> +#define AM33XX_GMII_SEL_MODE_RMII (1)
> +#define AM33XX_GMII_SEL_MODE_RGMII (2)
> +
> +#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN BIT(7)
> +#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN BIT(6)
These defined can go into cpsw.h so that we can maintain platform
defines in one file.
> +
> #define cpsw_enable_irq(priv) \
> do { \
> u32 i; \
> @@ -980,6 +987,56 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
> priv->host_port, ALE_VLAN, slave->port_vlan);
> }
>
> +static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
> + struct cpsw_priv *priv)
> +{
> + u32 reg, mask, mode = 0;
Please define each variable in separate line to make it clean.
Please check for IS_ERR(gmii_sel_reg) and return if gmii register is not
specified in DT then the below code will crash when trying to access
gmii_sel_reg.
> +
> + switch (priv->data.hw_type) {
> + case CPSW_TYPE_AM33XX:
> + if (!priv->gmii_sel_reg)
> + break;
> +
> + reg = readl(priv->gmii_sel_reg);
> +
> + if (slave->phy) {
> + switch (slave->phy->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + default:
> + mode = AM33XX_GMII_SEL_MODE_MII;
> + break;
> + case PHY_INTERFACE_MODE_RMII:
> + mode = AM33XX_GMII_SEL_MODE_RMII;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
You need to take care of other RGMII modes as well, for your info
AM335xevm phy mode is "rgmii-txid"
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Mugunthan V N @ 2013-08-23 16:45 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, nsekhar, sergei.shtylyov, davem,
ujhelyi.m, vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <52177052.1030308@ti.com>
On Friday 23 August 2013 07:53 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>> In order to support features that are specific to the AM335x IP, we have
>> to add hardware types and another compatible string.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>> Documentation/devicetree/bindings/net/cpsw.txt | 3 ++-
>> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++------
>> drivers/net/ethernet/ti/cpsw.h | 1 +
>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>> index 4e5ca54..b717458 100644
>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>> @@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>> ------------------------------------------------------
>>
>> Required properties:
>> -- compatible : Should be "ti,cpsw"
>> +- compatible : Should be "ti,cpsw" for generic cpsw support, or
>> + "ti,am3352-cpsw" for AM3352 SoCs
>> - reg : physical base address and size of the cpsw
>> registers map.
>> An optional third memory region can be supplied if
>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>> index 7a25ff4..73c44cb6 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -155,6 +155,11 @@ do { \
>> ((priv->data.dual_emac) ? priv->emac_port : \
>> priv->data.active_slave)
>>
>> +enum {
>> + CPSW_TYPE_GENERIC,
>> + CPSW_TYPE_AM33XX
>> +};
>> +
>> static int debug_level;
>> module_param(debug_level, int, 0);
>> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
>> @@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
>> slave->port_vlan = data->dual_emac_res_vlan;
>> }
>>
>> +static const struct of_device_id cpsw_of_mtable[] = {
>> + {
>> + .compatible = "ti,am3352-cpsw",
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.
>
>
But the same IP can be used in different SoC as well where the control
register may be different as per the Silicon Integration team's decision?
Ideally there should be a separate control module driver so that it can
take care of different SoC related needs.
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH v2 4/5] net: ethernet: cpsw: add support for hardware interface mode config
From: Mugunthan V N @ 2013-08-23 16:38 UTC (permalink / raw)
To: Daniel Mack
Cc: netdev, nsekhar, sergei.shtylyov, davem, ujhelyi.m, vaibhav.bedia,
d-gerlach, linux-arm-kernel, linux-omap, devicetree
In-Reply-To: <1377247417-27386-5-git-send-email-zonque@gmail.com>
On Friday 23 August 2013 02:13 PM, Daniel Mack wrote:
> The cpsw currently lacks code to properly set up the hardware interface
> mode on AM33xx. Other platforms might be equally affected.
>
> Usually, the bootloader will configure the control module register, so
> probably that's why such support wasn't needed in the past. In suspend
> mode though, this register is modified, and so it needs reprogramming
> after resume.
>
> This patch adds code that makes use of the previously added and optional
> support for passing the control mode register, and configures the
> correct register bits when the slave is opened.
>
> The AM33xx also has a bit for each slave to configure the RMII reference
> clock direction. Setting it is now supported by a per-slave DT property.
>
> This code path introducted by this patch is currently exclusive for
> am33xx.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
> Documentation/devicetree/bindings/net/cpsw.txt | 2 +
> drivers/net/ethernet/ti/cpsw.c | 61 ++++++++++++++++++++++++++
> drivers/net/ethernet/ti/cpsw.h | 1 +
> 3 files changed, 64 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index b717458..0895a51 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -34,6 +34,8 @@ Required properties:
> - phy_id : Specifies slave phy id
> - phy-mode : The interface between the SoC and the PHY (a string
> that of_get_phy_mode() can understand)
> +- ti,rmii-clock-ext : If present, the driver will configure the RMII
> + interface to external clock usage
> - mac-address : Specifies slave MAC address
>
> Optional properties:
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 2e19de0..6e36f49 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -138,6 +138,13 @@ do { \
> #define CPSW_CMINTMAX_INTVL (1000 / CPSW_CMINTMIN_CNT)
> #define CPSW_CMINTMIN_INTVL ((1000 / CPSW_CMINTMAX_CNT) + 1)
>
> +#define AM33XX_GMII_SEL_MODE_MII (0)
> +#define AM33XX_GMII_SEL_MODE_RMII (1)
> +#define AM33XX_GMII_SEL_MODE_RGMII (2)
> +
> +#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN BIT(7)
> +#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN BIT(6)
These defined can go into cpsw.h so that we can maintain platform
defines in one file.
> +
> #define cpsw_enable_irq(priv) \
> do { \
> u32 i; \
> @@ -980,6 +987,56 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
> priv->host_port, ALE_VLAN, slave->port_vlan);
> }
>
> +static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
> + struct cpsw_priv *priv)
> +{
> + u32 reg, mask, mode = 0;
> +
Please define each variable in separate line to make it clean.
Please check for IS_ERR(gmii_sel_reg) and return if gmii register is not
specified in DT then the below code will crash when trying to access
gmii_sel_reg.
> + switch (priv->data.hw_type) {
> + case CPSW_TYPE_AM33XX:
> + if (!priv->gmii_sel_reg)
> + break;
> +
> + reg = readl(priv->gmii_sel_reg);
> +
> + if (slave->phy) {
> + switch (slave->phy->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + default:
> + mode = AM33XX_GMII_SEL_MODE_MII;
> + break;
> + case PHY_INTERFACE_MODE_RMII:
> + mode = AM33XX_GMII_SEL_MODE_RMII;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
You need to take care of other RGMII modes as well, for your info
AM335xevm phy mode is "rgmii-txid"
Regards
Mugunthan V N
^ permalink raw reply
* [PATCH] VMXNET3: Add support for virtual IOMMU
From: Andy King @ 2013-08-23 16:33 UTC (permalink / raw)
To: netdev, linux-kernel, virtualization; +Cc: gregkh, davem, pv-drivers, Andy King
This patch adds support for virtual IOMMU to the vmxnet3 module. We
switch to DMA consistent mappings for anything we pass to the device.
There were a few places where we already did this, but using pci_blah();
these have been fixed to use dma_blah(), along with all new occurrences
where we've replaced kmalloc() and friends.
Also fix two small bugs:
1) use after free of rq->buf_info in vmxnet3_rq_destroy()
2) a cpu_to_le32() that should have been a cpu_to_le64()
Acked-by: George Zhang <georgezhang@vmware.com>
Acked-by: Aditya Sarwade <asarwade@vmware.com>
Signed-off-by: Andy King <acking@vmware.com>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 211 +++++++++++++++++++++++--------------
drivers/net/vmxnet3/vmxnet3_int.h | 10 ++-
2 files changed, 138 insertions(+), 83 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 55a62ca..7e2788c 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -313,10 +313,10 @@ vmxnet3_unmap_tx_buf(struct vmxnet3_tx_buf_info *tbi,
struct pci_dev *pdev)
{
if (tbi->map_type == VMXNET3_MAP_SINGLE)
- pci_unmap_single(pdev, tbi->dma_addr, tbi->len,
+ dma_unmap_single(&pdev->dev, tbi->dma_addr, tbi->len,
PCI_DMA_TODEVICE);
else if (tbi->map_type == VMXNET3_MAP_PAGE)
- pci_unmap_page(pdev, tbi->dma_addr, tbi->len,
+ dma_unmap_page(&pdev->dev, tbi->dma_addr, tbi->len,
PCI_DMA_TODEVICE);
else
BUG_ON(tbi->map_type != VMXNET3_MAP_NONE);
@@ -429,25 +429,29 @@ vmxnet3_tq_destroy(struct vmxnet3_tx_queue *tq,
struct vmxnet3_adapter *adapter)
{
if (tq->tx_ring.base) {
- pci_free_consistent(adapter->pdev, tq->tx_ring.size *
- sizeof(struct Vmxnet3_TxDesc),
- tq->tx_ring.base, tq->tx_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, tq->tx_ring.size *
+ sizeof(struct Vmxnet3_TxDesc),
+ tq->tx_ring.base, tq->tx_ring.basePA);
tq->tx_ring.base = NULL;
}
if (tq->data_ring.base) {
- pci_free_consistent(adapter->pdev, tq->data_ring.size *
- sizeof(struct Vmxnet3_TxDataDesc),
- tq->data_ring.base, tq->data_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, tq->data_ring.size *
+ sizeof(struct Vmxnet3_TxDataDesc),
+ tq->data_ring.base, tq->data_ring.basePA);
tq->data_ring.base = NULL;
}
if (tq->comp_ring.base) {
- pci_free_consistent(adapter->pdev, tq->comp_ring.size *
- sizeof(struct Vmxnet3_TxCompDesc),
- tq->comp_ring.base, tq->comp_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, tq->comp_ring.size *
+ sizeof(struct Vmxnet3_TxCompDesc),
+ tq->comp_ring.base, tq->comp_ring.basePA);
tq->comp_ring.base = NULL;
}
- kfree(tq->buf_info);
- tq->buf_info = NULL;
+ if (tq->buf_info) {
+ dma_free_coherent(&adapter->pdev->dev,
+ tq->tx_ring.size * sizeof(tq->buf_info[0]),
+ tq->buf_info, tq->buf_info_pa);
+ tq->buf_info = NULL;
+ }
}
@@ -496,37 +500,38 @@ static int
vmxnet3_tq_create(struct vmxnet3_tx_queue *tq,
struct vmxnet3_adapter *adapter)
{
+ size_t sz;
+
BUG_ON(tq->tx_ring.base || tq->data_ring.base ||
tq->comp_ring.base || tq->buf_info);
- tq->tx_ring.base = pci_alloc_consistent(adapter->pdev, tq->tx_ring.size
- * sizeof(struct Vmxnet3_TxDesc),
- &tq->tx_ring.basePA);
+ tq->tx_ring.base = dma_alloc_coherent(&adapter->pdev->dev,
+ tq->tx_ring.size * sizeof(struct Vmxnet3_TxDesc),
+ &tq->tx_ring.basePA, GFP_KERNEL);
if (!tq->tx_ring.base) {
netdev_err(adapter->netdev, "failed to allocate tx ring\n");
goto err;
}
- tq->data_ring.base = pci_alloc_consistent(adapter->pdev,
- tq->data_ring.size *
- sizeof(struct Vmxnet3_TxDataDesc),
- &tq->data_ring.basePA);
+ tq->data_ring.base = dma_alloc_coherent(&adapter->pdev->dev,
+ tq->data_ring.size * sizeof(struct Vmxnet3_TxDataDesc),
+ &tq->data_ring.basePA, GFP_KERNEL);
if (!tq->data_ring.base) {
netdev_err(adapter->netdev, "failed to allocate data ring\n");
goto err;
}
- tq->comp_ring.base = pci_alloc_consistent(adapter->pdev,
- tq->comp_ring.size *
- sizeof(struct Vmxnet3_TxCompDesc),
- &tq->comp_ring.basePA);
+ tq->comp_ring.base = dma_alloc_coherent(&adapter->pdev->dev,
+ tq->comp_ring.size * sizeof(struct Vmxnet3_TxCompDesc),
+ &tq->comp_ring.basePA, GFP_KERNEL);
if (!tq->comp_ring.base) {
netdev_err(adapter->netdev, "failed to allocate tx comp ring\n");
goto err;
}
- tq->buf_info = kcalloc(tq->tx_ring.size, sizeof(tq->buf_info[0]),
- GFP_KERNEL);
+ sz = tq->tx_ring.size * sizeof(tq->buf_info[0]);
+ tq->buf_info = dma_zalloc_coherent(&adapter->pdev->dev, sz,
+ &tq->buf_info_pa, GFP_KERNEL);
if (!tq->buf_info)
goto err;
@@ -578,7 +583,8 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
break;
}
- rbi->dma_addr = pci_map_single(adapter->pdev,
+ rbi->dma_addr = dma_map_single(
+ &adapter->pdev->dev,
rbi->skb->data, rbi->len,
PCI_DMA_FROMDEVICE);
} else {
@@ -595,7 +601,8 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
rq->stats.rx_buf_alloc_failure++;
break;
}
- rbi->dma_addr = pci_map_page(adapter->pdev,
+ rbi->dma_addr = dma_map_page(
+ &adapter->pdev->dev,
rbi->page, 0, PAGE_SIZE,
PCI_DMA_FROMDEVICE);
} else {
@@ -705,7 +712,7 @@ vmxnet3_map_pkt(struct sk_buff *skb, struct vmxnet3_tx_ctx *ctx,
tbi = tq->buf_info + tq->tx_ring.next2fill;
tbi->map_type = VMXNET3_MAP_SINGLE;
- tbi->dma_addr = pci_map_single(adapter->pdev,
+ tbi->dma_addr = dma_map_single(&adapter->pdev->dev,
skb->data + buf_offset, buf_size,
PCI_DMA_TODEVICE);
@@ -1221,7 +1228,8 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
goto rcd_done;
}
- pci_unmap_single(adapter->pdev, rbi->dma_addr, rbi->len,
+ dma_unmap_single(&adapter->pdev->dev, rbi->dma_addr,
+ rbi->len,
PCI_DMA_FROMDEVICE);
#ifdef VMXNET3_RSS
@@ -1233,7 +1241,7 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
/* Immediate refill */
rbi->skb = new_skb;
- rbi->dma_addr = pci_map_single(adapter->pdev,
+ rbi->dma_addr = dma_map_single(&adapter->pdev->dev,
rbi->skb->data, rbi->len,
PCI_DMA_FROMDEVICE);
rxd->addr = cpu_to_le64(rbi->dma_addr);
@@ -1267,7 +1275,7 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
}
if (rcd->len) {
- pci_unmap_page(adapter->pdev,
+ dma_unmap_page(&adapter->pdev->dev,
rbi->dma_addr, rbi->len,
PCI_DMA_FROMDEVICE);
@@ -1276,7 +1284,8 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
/* Immediate refill */
rbi->page = new_page;
- rbi->dma_addr = pci_map_page(adapter->pdev, rbi->page,
+ rbi->dma_addr = dma_map_page(&adapter->pdev->dev,
+ rbi->page,
0, PAGE_SIZE,
PCI_DMA_FROMDEVICE);
rxd->addr = cpu_to_le64(rbi->dma_addr);
@@ -1352,13 +1361,13 @@ vmxnet3_rq_cleanup(struct vmxnet3_rx_queue *rq,
if (rxd->btype == VMXNET3_RXD_BTYPE_HEAD &&
rq->buf_info[ring_idx][i].skb) {
- pci_unmap_single(adapter->pdev, rxd->addr,
+ dma_unmap_single(&adapter->pdev->dev, rxd->addr,
rxd->len, PCI_DMA_FROMDEVICE);
dev_kfree_skb(rq->buf_info[ring_idx][i].skb);
rq->buf_info[ring_idx][i].skb = NULL;
} else if (rxd->btype == VMXNET3_RXD_BTYPE_BODY &&
rq->buf_info[ring_idx][i].page) {
- pci_unmap_page(adapter->pdev, rxd->addr,
+ dma_unmap_page(&adapter->pdev->dev, rxd->addr,
rxd->len, PCI_DMA_FROMDEVICE);
put_page(rq->buf_info[ring_idx][i].page);
rq->buf_info[ring_idx][i].page = NULL;
@@ -1400,25 +1409,31 @@ static void vmxnet3_rq_destroy(struct vmxnet3_rx_queue *rq,
}
- kfree(rq->buf_info[0]);
-
for (i = 0; i < 2; i++) {
if (rq->rx_ring[i].base) {
- pci_free_consistent(adapter->pdev, rq->rx_ring[i].size
- * sizeof(struct Vmxnet3_RxDesc),
- rq->rx_ring[i].base,
- rq->rx_ring[i].basePA);
+ dma_free_coherent(&adapter->pdev->dev,
+ rq->rx_ring[i].size
+ * sizeof(struct Vmxnet3_RxDesc),
+ rq->rx_ring[i].base,
+ rq->rx_ring[i].basePA);
rq->rx_ring[i].base = NULL;
}
rq->buf_info[i] = NULL;
}
if (rq->comp_ring.base) {
- pci_free_consistent(adapter->pdev, rq->comp_ring.size *
- sizeof(struct Vmxnet3_RxCompDesc),
- rq->comp_ring.base, rq->comp_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, rq->comp_ring.size
+ * sizeof(struct Vmxnet3_RxCompDesc),
+ rq->comp_ring.base, rq->comp_ring.basePA);
rq->comp_ring.base = NULL;
}
+
+ if (rq->buf_info[0]) {
+ size_t sz = sizeof(struct vmxnet3_rx_buf_info) *
+ (rq->rx_ring[0].size + rq->rx_ring[1].size);
+ dma_free_coherent(&adapter->pdev->dev, sz, rq->buf_info[0],
+ rq->buf_info_pa);
+ }
}
@@ -1503,8 +1518,10 @@ vmxnet3_rq_create(struct vmxnet3_rx_queue *rq, struct vmxnet3_adapter *adapter)
for (i = 0; i < 2; i++) {
sz = rq->rx_ring[i].size * sizeof(struct Vmxnet3_RxDesc);
- rq->rx_ring[i].base = pci_alloc_consistent(adapter->pdev, sz,
- &rq->rx_ring[i].basePA);
+ rq->rx_ring[i].base = dma_alloc_coherent(
+ &adapter->pdev->dev, sz,
+ &rq->rx_ring[i].basePA,
+ GFP_KERNEL);
if (!rq->rx_ring[i].base) {
netdev_err(adapter->netdev,
"failed to allocate rx ring %d\n", i);
@@ -1513,8 +1530,9 @@ vmxnet3_rq_create(struct vmxnet3_rx_queue *rq, struct vmxnet3_adapter *adapter)
}
sz = rq->comp_ring.size * sizeof(struct Vmxnet3_RxCompDesc);
- rq->comp_ring.base = pci_alloc_consistent(adapter->pdev, sz,
- &rq->comp_ring.basePA);
+ rq->comp_ring.base = dma_alloc_coherent(&adapter->pdev->dev, sz,
+ &rq->comp_ring.basePA,
+ GFP_KERNEL);
if (!rq->comp_ring.base) {
netdev_err(adapter->netdev, "failed to allocate rx comp ring\n");
goto err;
@@ -1522,7 +1540,8 @@ vmxnet3_rq_create(struct vmxnet3_rx_queue *rq, struct vmxnet3_adapter *adapter)
sz = sizeof(struct vmxnet3_rx_buf_info) * (rq->rx_ring[0].size +
rq->rx_ring[1].size);
- bi = kzalloc(sz, GFP_KERNEL);
+ bi = dma_zalloc_coherent(&adapter->pdev->dev, sz, &rq->buf_info_pa,
+ GFP_KERNEL);
if (!bi)
goto err;
@@ -2005,6 +2024,7 @@ vmxnet3_set_mc(struct net_device *netdev)
struct Vmxnet3_RxFilterConf *rxConf =
&adapter->shared->devRead.rxFilterConf;
u8 *new_table = NULL;
+ dma_addr_t new_table_pa = 0;
u32 new_mode = VMXNET3_RXM_UCAST;
if (netdev->flags & IFF_PROMISC) {
@@ -2028,8 +2048,12 @@ vmxnet3_set_mc(struct net_device *netdev)
new_mode |= VMXNET3_RXM_MCAST;
rxConf->mfTableLen = cpu_to_le16(
netdev_mc_count(netdev) * ETH_ALEN);
- rxConf->mfTablePA = cpu_to_le64(virt_to_phys(
- new_table));
+ new_table_pa = dma_map_single(
+ &adapter->pdev->dev,
+ new_table,
+ rxConf->mfTableLen,
+ PCI_DMA_TODEVICE);
+ rxConf->mfTablePA = cpu_to_le64(new_table_pa);
} else {
netdev_info(netdev, "failed to copy mcast list"
", setting ALL_MULTI\n");
@@ -2056,7 +2080,11 @@ vmxnet3_set_mc(struct net_device *netdev)
VMXNET3_CMD_UPDATE_MAC_FILTERS);
spin_unlock_irqrestore(&adapter->cmd_lock, flags);
- kfree(new_table);
+ if (new_table) {
+ dma_unmap_single(&adapter->pdev->dev, new_table_pa,
+ rxConf->mfTableLen, PCI_DMA_TODEVICE);
+ kfree(new_table);
+ }
}
void
@@ -2096,7 +2124,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
devRead->misc.driverInfo.vmxnet3RevSpt = cpu_to_le32(1);
devRead->misc.driverInfo.uptVerSpt = cpu_to_le32(1);
- devRead->misc.ddPA = cpu_to_le64(virt_to_phys(adapter));
+ devRead->misc.ddPA = cpu_to_le64(adapter->adapter_pa);
devRead->misc.ddLen = cpu_to_le32(sizeof(struct vmxnet3_adapter));
/* set up feature flags */
@@ -2125,7 +2153,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
tqc->txRingBasePA = cpu_to_le64(tq->tx_ring.basePA);
tqc->dataRingBasePA = cpu_to_le64(tq->data_ring.basePA);
tqc->compRingBasePA = cpu_to_le64(tq->comp_ring.basePA);
- tqc->ddPA = cpu_to_le64(virt_to_phys(tq->buf_info));
+ tqc->ddPA = cpu_to_le64(tq->buf_info_pa);
tqc->txRingSize = cpu_to_le32(tq->tx_ring.size);
tqc->dataRingSize = cpu_to_le32(tq->data_ring.size);
tqc->compRingSize = cpu_to_le32(tq->comp_ring.size);
@@ -2143,8 +2171,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
rqc->rxRingBasePA[0] = cpu_to_le64(rq->rx_ring[0].basePA);
rqc->rxRingBasePA[1] = cpu_to_le64(rq->rx_ring[1].basePA);
rqc->compRingBasePA = cpu_to_le64(rq->comp_ring.basePA);
- rqc->ddPA = cpu_to_le64(virt_to_phys(
- rq->buf_info));
+ rqc->ddPA = cpu_to_le64(rq->buf_info_pa);
rqc->rxRingSize[0] = cpu_to_le32(rq->rx_ring[0].size);
rqc->rxRingSize[1] = cpu_to_le32(rq->rx_ring[1].size);
rqc->compRingSize = cpu_to_le32(rq->comp_ring.size);
@@ -2184,8 +2211,9 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
i, adapter->num_rx_queues);
devRead->rssConfDesc.confVer = 1;
- devRead->rssConfDesc.confLen = sizeof(*rssConf);
- devRead->rssConfDesc.confPA = virt_to_phys(rssConf);
+ devRead->rssConfDesc.confLen = cpu_to_le32(sizeof(*rssConf));
+ devRead->rssConfDesc.confPA =
+ cpu_to_le64(adapter->rss_conf_pa);
}
#endif /* VMXNET3_RSS */
@@ -2948,9 +2976,13 @@ vmxnet3_probe_device(struct pci_dev *pdev,
adapter->pdev = pdev;
spin_lock_init(&adapter->cmd_lock);
- adapter->shared = pci_alloc_consistent(adapter->pdev,
- sizeof(struct Vmxnet3_DriverShared),
- &adapter->shared_pa);
+ adapter->adapter_pa = dma_map_single(&adapter->pdev->dev, adapter,
+ sizeof(struct vmxnet3_adapter),
+ PCI_DMA_TODEVICE);
+ adapter->shared = dma_alloc_coherent(
+ &adapter->pdev->dev,
+ sizeof(struct Vmxnet3_DriverShared),
+ &adapter->shared_pa, GFP_KERNEL);
if (!adapter->shared) {
dev_err(&pdev->dev, "Failed to allocate memory\n");
err = -ENOMEM;
@@ -2963,8 +2995,9 @@ vmxnet3_probe_device(struct pci_dev *pdev,
size = sizeof(struct Vmxnet3_TxQueueDesc) * adapter->num_tx_queues;
size += sizeof(struct Vmxnet3_RxQueueDesc) * adapter->num_rx_queues;
- adapter->tqd_start = pci_alloc_consistent(adapter->pdev, size,
- &adapter->queue_desc_pa);
+ adapter->tqd_start = dma_alloc_coherent(&adapter->pdev->dev, size,
+ &adapter->queue_desc_pa,
+ GFP_KERNEL);
if (!adapter->tqd_start) {
dev_err(&pdev->dev, "Failed to allocate memory\n");
@@ -2974,7 +3007,10 @@ vmxnet3_probe_device(struct pci_dev *pdev,
adapter->rqd_start = (struct Vmxnet3_RxQueueDesc *)(adapter->tqd_start +
adapter->num_tx_queues);
- adapter->pm_conf = kmalloc(sizeof(struct Vmxnet3_PMConf), GFP_KERNEL);
+ adapter->pm_conf = dma_alloc_coherent(&adapter->pdev->dev,
+ sizeof(struct Vmxnet3_PMConf),
+ &adapter->pm_conf_pa,
+ GFP_KERNEL);
if (adapter->pm_conf == NULL) {
err = -ENOMEM;
goto err_alloc_pm;
@@ -2982,7 +3018,10 @@ vmxnet3_probe_device(struct pci_dev *pdev,
#ifdef VMXNET3_RSS
- adapter->rss_conf = kmalloc(sizeof(struct UPT1_RSSConf), GFP_KERNEL);
+ adapter->rss_conf = dma_alloc_coherent(&adapter->pdev->dev,
+ sizeof(struct UPT1_RSSConf),
+ &adapter->rss_conf_pa,
+ GFP_KERNEL);
if (adapter->rss_conf == NULL) {
err = -ENOMEM;
goto err_alloc_rss;
@@ -3077,17 +3116,22 @@ err_ver:
vmxnet3_free_pci_resources(adapter);
err_alloc_pci:
#ifdef VMXNET3_RSS
- kfree(adapter->rss_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct UPT1_RSSConf),
+ adapter->rss_conf, adapter->rss_conf_pa);
err_alloc_rss:
#endif
- kfree(adapter->pm_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct Vmxnet3_PMConf),
+ adapter->pm_conf, adapter->pm_conf_pa);
err_alloc_pm:
- pci_free_consistent(adapter->pdev, size, adapter->tqd_start,
- adapter->queue_desc_pa);
+ dma_free_coherent(&adapter->pdev->dev, size, adapter->tqd_start,
+ adapter->queue_desc_pa);
err_alloc_queue_desc:
- pci_free_consistent(adapter->pdev, sizeof(struct Vmxnet3_DriverShared),
- adapter->shared, adapter->shared_pa);
+ dma_free_coherent(&adapter->pdev->dev,
+ sizeof(struct Vmxnet3_DriverShared),
+ adapter->shared, adapter->shared_pa);
err_alloc_shared:
+ dma_unmap_single(&adapter->pdev->dev, adapter->adapter_pa,
+ sizeof(struct vmxnet3_adapter), PCI_DMA_TODEVICE);
pci_set_drvdata(pdev, NULL);
free_netdev(netdev);
return err;
@@ -3118,16 +3162,21 @@ vmxnet3_remove_device(struct pci_dev *pdev)
vmxnet3_free_intr_resources(adapter);
vmxnet3_free_pci_resources(adapter);
#ifdef VMXNET3_RSS
- kfree(adapter->rss_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct UPT1_RSSConf),
+ adapter->rss_conf, adapter->rss_conf_pa);
#endif
- kfree(adapter->pm_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct Vmxnet3_PMConf),
+ adapter->pm_conf, adapter->pm_conf_pa);
size = sizeof(struct Vmxnet3_TxQueueDesc) * adapter->num_tx_queues;
size += sizeof(struct Vmxnet3_RxQueueDesc) * num_rx_queues;
- pci_free_consistent(adapter->pdev, size, adapter->tqd_start,
- adapter->queue_desc_pa);
- pci_free_consistent(adapter->pdev, sizeof(struct Vmxnet3_DriverShared),
- adapter->shared, adapter->shared_pa);
+ dma_free_coherent(&adapter->pdev->dev, size, adapter->tqd_start,
+ adapter->queue_desc_pa);
+ dma_free_coherent(&adapter->pdev->dev,
+ sizeof(struct Vmxnet3_DriverShared),
+ adapter->shared, adapter->shared_pa);
+ dma_unmap_single(&adapter->pdev->dev, adapter->adapter_pa,
+ sizeof(struct vmxnet3_adapter), PCI_DMA_TODEVICE);
free_netdev(netdev);
}
@@ -3227,8 +3276,8 @@ skip_arp:
adapter->shared->devRead.pmConfDesc.confVer = cpu_to_le32(1);
adapter->shared->devRead.pmConfDesc.confLen = cpu_to_le32(sizeof(
*pmConf));
- adapter->shared->devRead.pmConfDesc.confPA = cpu_to_le64(virt_to_phys(
- pmConf));
+ adapter->shared->devRead.pmConfDesc.confPA =
+ cpu_to_le64(adapter->pm_conf_pa);
spin_lock_irqsave(&adapter->cmd_lock, flags);
VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
@@ -3265,8 +3314,8 @@ vmxnet3_resume(struct device *device)
adapter->shared->devRead.pmConfDesc.confVer = cpu_to_le32(1);
adapter->shared->devRead.pmConfDesc.confLen = cpu_to_le32(sizeof(
*pmConf));
- adapter->shared->devRead.pmConfDesc.confPA = cpu_to_le64(virt_to_phys(
- pmConf));
+ adapter->shared->devRead.pmConfDesc.confPA =
+ cpu_to_le64(adapter->pm_conf_pa);
netif_device_attach(netdev);
pci_set_power_state(pdev, PCI_D0);
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 3541814..a03f358 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -70,10 +70,10 @@
/*
* Version numbers
*/
-#define VMXNET3_DRIVER_VERSION_STRING "1.1.30.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING "1.2.0.0-k"
/* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM 0x01011E00
+#define VMXNET3_DRIVER_VERSION_NUM 0x01020000
#if defined(CONFIG_PCI_MSI)
/* RSS only makes sense if MSI-X is supported. */
@@ -229,6 +229,7 @@ struct vmxnet3_tx_queue {
spinlock_t tx_lock;
struct vmxnet3_cmd_ring tx_ring;
struct vmxnet3_tx_buf_info *buf_info;
+ dma_addr_t buf_info_pa;
struct vmxnet3_tx_data_ring data_ring;
struct vmxnet3_comp_ring comp_ring;
struct Vmxnet3_TxQueueCtrl *shared;
@@ -277,6 +278,7 @@ struct vmxnet3_rx_queue {
u32 qid; /* rqID in RCD for buffer from 1st ring */
u32 qid2; /* rqID in RCD for buffer from 2nd ring */
struct vmxnet3_rx_buf_info *buf_info[2];
+ dma_addr_t buf_info_pa;
struct Vmxnet3_RxQueueCtrl *shared;
struct vmxnet3_rq_driver_stats stats;
} __attribute__((__aligned__(SMP_CACHE_BYTES)));
@@ -353,6 +355,10 @@ struct vmxnet3_adapter {
unsigned long state; /* VMXNET3_STATE_BIT_xxx */
int share_intr;
+
+ dma_addr_t adapter_pa;
+ dma_addr_t pm_conf_pa;
+ dma_addr_t rss_conf_pa;
};
#define VMXNET3_WRITE_BAR0_REG(adapter, reg, val) \
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Sekhar Nori @ 2013-08-23 16:31 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52177052.1030308@ti.com>
On 8/23/2013 7:53 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>> In order to support features that are specific to the AM335x IP, we have
>> to add hardware types and another compatible string.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>> Documentation/devicetree/bindings/net/cpsw.txt | 3 ++-
>> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++------
>> drivers/net/ethernet/ti/cpsw.h | 1 +
>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>> index 4e5ca54..b717458 100644
>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>> @@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>> ------------------------------------------------------
>>
>> Required properties:
>> -- compatible : Should be "ti,cpsw"
>> +- compatible : Should be "ti,cpsw" for generic cpsw support, or
>> + "ti,am3352-cpsw" for AM3352 SoCs
>> - reg : physical base address and size of the cpsw
>> registers map.
>> An optional third memory region can be supplied if
>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>> index 7a25ff4..73c44cb6 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -155,6 +155,11 @@ do { \
>> ((priv->data.dual_emac) ? priv->emac_port : \
>> priv->data.active_slave)
>>
>> +enum {
>> + CPSW_TYPE_GENERIC,
>> + CPSW_TYPE_AM33XX
>> +};
>> +
>> static int debug_level;
>> module_param(debug_level, int, 0);
>> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
>> @@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
>> slave->port_vlan = data->dual_emac_res_vlan;
>> }
>>
>> +static const struct of_device_id cpsw_of_mtable[] = {
>> + {
>> + .compatible = "ti,am3352-cpsw",
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.
The objection then should not just be to the name of compatible string,
but to handling AM335x control module register in CPSW driver. What
would be the alternate method of doing what Daniel is doing? I vaguely
remember there were attempts to develop an independent control module
driver that were abandoned.
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Daniel Mack @ 2013-08-23 16:30 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: netdev, bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52177052.1030308@ti.com>
On 23.08.2013 16:23, Santosh Shilimkar wrote:
> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>> +static const struct of_device_id cpsw_of_mtable[] = {
>> + {
>> + .compatible = "ti,am3352-cpsw",
>
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.
As I already mentioned in the cover letter and in the commit message, I
just don't know which criteria makes most sense here.
On a general note, I would say that chances that this exactly IP core
with the same version number will appear on some other silicon which
doesn't support the control mode register in an AM33xx fashion, is not
necessarily negligible.
So what that new compatible string denotes is the cpsw in a version as
found on am3352 SoCs, which is actually exactly what it does.
I don't have a strong opinion here, but see your point. I just don't
have a better idea on how to treat that.
Daniel
^ permalink raw reply
* Re: [PATCH v4 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Daniel Mack @ 2013-08-23 16:21 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: netdev, bcousson, nsekhar, davem, ujhelyi.m, mugunthanvnm,
vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <521778DB.7040402@cogentembedded.com>
On 23.08.2013 16:59, Sergei Shtylyov wrote:
> On 23-08-2013 18:16, Daniel Mack wrote:
>> + priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(priv->gmii_sel_reg)) {
>> + dev_err(priv->dev, "unable to map control i/o region\n");
>
> You didn't actually seem to heed my words about error message.
Well yes I did, but only in the check for platform_get_resource(). As
the comment says - we pass on if that memory region is not given, but if
it is given, it also has to be valid.
> And don't you want to do:
>
> res = PTR_ERR(priv->gmii_sel_reg);
Erm, of course. Sorry for that.
Daniel
^ permalink raw reply
* pull request: wireless 2013-08-23
From: John W. Linville @ 2013-08-23 16:20 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 11214 bytes --]
Dave,
This is one more set of fixes intended for the 3.11 stream...
For the mac80211 bits, Johannes says:
"I have three more patches for the 3.11 stream: Felix's fix for the
fairly visible brcmsmac crash, a fix from Simon for an IBSS join bug I
found and a fix for a channel context bug in IBSS I'd introduced."
Along with those...
Sujith Manoharan makes a minor change to not use a PLL hang workaroun
for AR9550. This one-liner fixes a couple of bugs reported in the Red Hat
bugzilla.
Helmut Schaa addresses an ath9k_htc bug that mangles frame headers
during Tx. This fix is small, tested by the bug reported and isolated
to ath9k_htc.
Stanislaw Gruszka reverts a recent iwl4965 change that broke rfkill
notification to user space.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit 4a5a8aa6c966eafc106543bd955ae388230420e5:
ipv4: expose IPV4_DEVCONF (2013-08-22 20:30:15 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
for you to fetch changes up to 81ca2ff9450c2e983ee416eca17ebe6fa9c3da4f:
Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless into for-davem (2013-08-23 11:47:48 -0400)
----------------------------------------------------------------
Felix Fietkau (1):
mac80211: add a flag to indicate CCK support for HT clients
Helmut Schaa (1):
ath9k_htc: Restore skb headroom when returning skb to mac80211
Johannes Berg (1):
mac80211: add missing channel context release
John W. Linville (2):
Merge branch 'for-john' of git://git.kernel.org/.../jberg/mac80211
Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem
Simon Wunderlich (1):
mac80211: ibss: fix ignored channel parameter
Stanislaw Gruszka (1):
iwl4965: fix rfkill set state regression
Sujith Manoharan (1):
ath9k: Enable PLL fix only for AR9340/AR9330
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 10 ++++++++
drivers/net/wireless/ath/ath9k/init.c | 3 ++-
drivers/net/wireless/ath/ath9k/main.c | 3 +--
drivers/net/wireless/ath/carl9170/main.c | 3 ++-
drivers/net/wireless/iwlegacy/4965-mac.c | 2 +-
drivers/net/wireless/rt2x00/rt2800lib.c | 3 ++-
include/net/mac80211.h | 1 +
net/mac80211/ibss.c | 34 +++++++++++++++++++++++----
net/mac80211/rc80211_minstrel_ht.c | 3 +++
9 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index e602c95..c028df7 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -448,6 +448,7 @@ static void ath9k_htc_tx_process(struct ath9k_htc_priv *priv,
struct ieee80211_conf *cur_conf = &priv->hw->conf;
bool txok;
int slot;
+ int hdrlen, padsize;
slot = strip_drv_header(priv, skb);
if (slot < 0) {
@@ -504,6 +505,15 @@ send_mac80211:
ath9k_htc_tx_clear_slot(priv, slot);
+ /* Remove padding before handing frame back to mac80211 */
+ hdrlen = ieee80211_get_hdrlen_from_skb(skb);
+
+ padsize = hdrlen & 3;
+ if (padsize && skb->len > hdrlen + padsize) {
+ memmove(skb->data + padsize, skb->data, hdrlen);
+ skb_pull(skb, padsize);
+ }
+
/* Send status to mac80211 */
ieee80211_tx_status(priv->hw, skb);
}
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 16f8b20..026a2a0 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -802,7 +802,8 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
IEEE80211_HW_PS_NULLFUNC_STACK |
IEEE80211_HW_SPECTRUM_MGMT |
IEEE80211_HW_REPORTS_TX_ACK_STATUS |
- IEEE80211_HW_SUPPORTS_RC_TABLE;
+ IEEE80211_HW_SUPPORTS_RC_TABLE |
+ IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
hw->flags |= IEEE80211_HW_AMPDU_AGGREGATION;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 1737a3e..cb5a655 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -173,8 +173,7 @@ static void ath_restart_work(struct ath_softc *sc)
{
ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
- if (AR_SREV_9340(sc->sc_ah) || AR_SREV_9485(sc->sc_ah) ||
- AR_SREV_9550(sc->sc_ah))
+ if (AR_SREV_9340(sc->sc_ah) || AR_SREV_9330(sc->sc_ah))
ieee80211_queue_delayed_work(sc->hw, &sc->hw_pll_work,
msecs_to_jiffies(ATH_PLL_WORK_INTERVAL));
diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index 4a33c6e..349fa22 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1860,7 +1860,8 @@ void *carl9170_alloc(size_t priv_size)
IEEE80211_HW_PS_NULLFUNC_STACK |
IEEE80211_HW_NEED_DTIM_BEFORE_ASSOC |
IEEE80211_HW_SUPPORTS_RC_TABLE |
- IEEE80211_HW_SIGNAL_DBM;
+ IEEE80211_HW_SIGNAL_DBM |
+ IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
if (!modparam_noht) {
/*
diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index f2ed62e..7acf5ee 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -4464,9 +4464,9 @@ il4965_irq_tasklet(struct il_priv *il)
set_bit(S_RFKILL, &il->status);
} else {
clear_bit(S_RFKILL, &il->status);
- wiphy_rfkill_set_hw_state(il->hw->wiphy, hw_rf_kill);
il_force_reset(il, true);
}
+ wiphy_rfkill_set_hw_state(il->hw->wiphy, hw_rf_kill);
handled |= CSR_INT_BIT_RF_KILL;
}
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 1f80ea5..1b41c8e 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -6133,7 +6133,8 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK |
IEEE80211_HW_AMPDU_AGGREGATION |
- IEEE80211_HW_REPORTS_TX_ACK_STATUS;
+ IEEE80211_HW_REPORTS_TX_ACK_STATUS |
+ IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
/*
* Don't set IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING for USB devices
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5b7a3da..551ba6a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1499,6 +1499,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_RC_TABLE = 1<<24,
IEEE80211_HW_P2P_DEV_ADDR_FOR_INTF = 1<<25,
IEEE80211_HW_TIMING_BEACON_ONLY = 1<<26,
+ IEEE80211_HW_SUPPORTS_HT_CCK_RATES = 1<<27,
};
/**
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index ea7b9c2..2d45643 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -36,7 +36,7 @@
static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
const u8 *bssid, const int beacon_int,
- struct ieee80211_channel *chan,
+ struct cfg80211_chan_def *req_chandef,
const u32 basic_rates,
const u16 capability, u64 tsf,
bool creator)
@@ -51,6 +51,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
u32 bss_change;
u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
struct cfg80211_chan_def chandef;
+ struct ieee80211_channel *chan;
struct beacon_data *presp;
int frame_len;
@@ -81,7 +82,9 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
sdata->drop_unencrypted = capability & WLAN_CAPABILITY_PRIVACY ? 1 : 0;
- chandef = ifibss->chandef;
+ /* make a copy of the chandef, it could be modified below. */
+ chandef = *req_chandef;
+ chan = chandef.chan;
if (!cfg80211_reg_can_beacon(local->hw.wiphy, &chandef)) {
chandef.width = NL80211_CHAN_WIDTH_20;
chandef.center_freq1 = chan->center_freq;
@@ -259,10 +262,12 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
struct cfg80211_bss *cbss =
container_of((void *)bss, struct cfg80211_bss, priv);
struct ieee80211_supported_band *sband;
+ struct cfg80211_chan_def chandef;
u32 basic_rates;
int i, j;
u16 beacon_int = cbss->beacon_interval;
const struct cfg80211_bss_ies *ies;
+ enum nl80211_channel_type chan_type;
u64 tsf;
sdata_assert_lock(sdata);
@@ -270,6 +275,26 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
if (beacon_int < 10)
beacon_int = 10;
+ switch (sdata->u.ibss.chandef.width) {
+ case NL80211_CHAN_WIDTH_20_NOHT:
+ case NL80211_CHAN_WIDTH_20:
+ case NL80211_CHAN_WIDTH_40:
+ chan_type = cfg80211_get_chandef_type(&sdata->u.ibss.chandef);
+ cfg80211_chandef_create(&chandef, cbss->channel, chan_type);
+ break;
+ case NL80211_CHAN_WIDTH_5:
+ case NL80211_CHAN_WIDTH_10:
+ cfg80211_chandef_create(&chandef, cbss->channel,
+ NL80211_CHAN_WIDTH_20_NOHT);
+ chandef.width = sdata->u.ibss.chandef.width;
+ break;
+ default:
+ /* fall back to 20 MHz for unsupported modes */
+ cfg80211_chandef_create(&chandef, cbss->channel,
+ NL80211_CHAN_WIDTH_20_NOHT);
+ break;
+ }
+
sband = sdata->local->hw.wiphy->bands[cbss->channel->band];
basic_rates = 0;
@@ -294,7 +319,7 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
__ieee80211_sta_join_ibss(sdata, cbss->bssid,
beacon_int,
- cbss->channel,
+ &chandef,
basic_rates,
cbss->capability,
tsf, false);
@@ -736,7 +761,7 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
sdata->drop_unencrypted = 0;
__ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
- ifibss->chandef.chan, ifibss->basic_rates,
+ &ifibss->chandef, ifibss->basic_rates,
capability, 0, true);
}
@@ -1138,6 +1163,7 @@ int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata)
clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED |
BSS_CHANGED_IBSS);
+ ieee80211_vif_release_channel(sdata);
synchronize_rcu();
kfree(presp);
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index f5aed96..f3bbea1 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -828,6 +828,9 @@ minstrel_ht_update_cck(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
if (sband->band != IEEE80211_BAND_2GHZ)
return;
+ if (!(mp->hw->flags & IEEE80211_HW_SUPPORTS_HT_CCK_RATES))
+ return;
+
mi->cck_supported = 0;
mi->cck_supported_short = 0;
for (i = 0; i < 4; i++) {
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
* Re: netem: the reorder discussion
From: Teco Boot @ 2013-08-23 16:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Stephen Hemminger, Ferry Huberts, Netem, netdev@vger.kernel.org,
core-users@pf.itd.nrl.navy.mil
In-Reply-To: <1377273838.8828.11.camel@edumazet-glaptop>
Op 23 aug. 2013 om 18:03 heeft Eric Dumazet <eric.dumazet@gmail.com> het volgende geschreven:
> On Fri, 2013-08-23 at 17:53 +0200, Teco Boot wrote:
>
>> Questions that pops out of of my head: What is more important,
>> understandable configuration or keep existing behavior for unmodified
>> configuration? Do we want to reorder, even with configured rate?
>>
>> My preference: make in understandable, document well and make all
>> sensible options available.
>
> The day 'rate' support was added, documentation became obsolete.
>
> Its actually hard to have both rate and delay + jitters + reorders or
> not.
>
> If you can fix this properly, patches are welcome, but I suspect you
> wanted to reintroduce the thing that made netem unusable for us.
No, for sure I do not want to make it unusable for you. Or anybody. The opposite is true.
I don't like the "look in souce" documentation. Or parameters with hard to understand side effects.
^ permalink raw reply
* Re: [PATCH] netfilter: avoid array overflow in nf_register_hook
From: Eric Dumazet @ 2013-08-23 16:05 UTC (permalink / raw)
To: Dong Fang
Cc: pablo, kaber, kadlec, davem, netfilter-devel, netfilter, coreteam,
netdev
In-Reply-To: <1377313495-16060-1-git-send-email-yp.fangdong@gmail.com>
On Fri, 2013-08-23 at 23:04 -0400, Dong Fang wrote:
> This patch fix the array overflow in nf_register_hook function
>
> Signed-off-by: Dong Fang <yp.fangdong@gmail.com>
> ---
> net/netfilter/core.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index 2217363..819eee1 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -68,8 +68,11 @@ static DEFINE_MUTEX(nf_hook_mutex);
> int nf_register_hook(struct nf_hook_ops *reg)
> {
> struct nf_hook_ops *elem;
> - int err;
> + int err = -EINVAL;
>
> + if (reg->pf >= NFPROTO_NUMPROTO || reg->hooknum >= NF_MAX_HOOKS)
> + return err;
> +
Thats a bug in the caller.
We want a hard crash so that the poor netfilter guy who made this error
can fix the bug before sending us a buggy patch.
^ permalink raw reply
* Re: netem: the reorder discussion
From: Eric Dumazet @ 2013-08-23 16:03 UTC (permalink / raw)
To: Teco Boot
Cc: Stephen Hemminger, Ferry Huberts, Netem, netdev@vger.kernel.org,
core-users@pf.itd.nrl.navy.mil
In-Reply-To: <31E9BEA1-3DE2-4DA7-9CEE-6B7C9C99F5EB@inf-net.nl>
On Fri, 2013-08-23 at 17:53 +0200, Teco Boot wrote:
> Questions that pops out of of my head: What is more important,
> understandable configuration or keep existing behavior for unmodified
> configuration? Do we want to reorder, even with configured rate?
>
> My preference: make in understandable, document well and make all
> sensible options available.
The day 'rate' support was added, documentation became obsolete.
Its actually hard to have both rate and delay + jitters + reorders or
not.
If you can fix this properly, patches are welcome, but I suspect you
wanted to reintroduce the thing that made netem unusable for us.
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 15:54 UTC (permalink / raw)
To: Benoit Cousson
Cc: Daniel Mack, netdev, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52177E29.6040103@baylibre.com>
On Friday 23 August 2013 11:22 AM, Benoit Cousson wrote:
> Hi Santosh,
>
> [...]
>
>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>> + {
>>> + .compatible = "ti,am3352-cpsw",
>> I didn't notice this earlier, but can't you use the IP version
>> as a compatible instead of using a SOC name. Whats really SOC specific
>> on this IP ? Sorry i have missed any earlier discussion on this but
>> this approach doesn't seem good. Its like adding SOC checks in the
>> driver subsystem.
>
> Hehe, welcome to the club!
> I keep arguing about that as well :-)
>
> I think we should create a Facebook group: "against-the-soc-version-in-the-compatible-string".
>
Indeed !!
Regards,
Santosh
^ permalink raw reply
* Re: netem: the reorder discussion
From: Teco Boot @ 2013-08-23 15:53 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Eric Dumazet, Ferry Huberts, Netem, netdev@vger.kernel.org,
core-users@pf.itd.nrl.navy.mil
In-Reply-To: <20130823084450.6baf2835@nehalam.linuxnetplumber.net>
Op 23 aug. 2013, om 17:44 heeft Stephen Hemminger <stephen@networkplumber.org> het volgende geschreven:
> On Fri, 23 Aug 2013 08:12:36 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> On Fri, 2013-08-23 at 16:16 +0200, Teco Boot wrote:
>>> I had reorder problems with netem qdisc. I make use of the NRL CORE
>>> network emulator, which sets up virtual routers and links, build with
>>> network namespaces and netem. Typical CORE config for a n1<-->n2 link
>>> with rate (1024Kbps), delay (50ms) and no jitter:
>>> qdisc tbf 1: dev n1.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
>>> lat 488.6ms
>>> qdisc netem 10: dev n1.eth0.222 parent 1:1 limit 1000 delay 50.0ms
>>> qdisc tbf 1: dev n2.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
>>> lat 488.6ms
>>> qdisc netem 10: dev n2.eth0.222 parent 1:1 limit 1000 delay 50.0ms
>>>
>>> I added jitter to this bi-directional link, e.g. 20ms. Now the delay
>>> for each packet is 50ms +/- 20ms is 30ms to 70ms.
>>> However, this has some unexpected results: packets may be reordered.
>>> That is because the actual delay is calculated for each packet. Some
>>> packets have a larger delay, some have a smaller. If the inter-packet
>>> spacing is smaller than the time differences set by netem (up to 2x
>>> configured jitter), packets are reordered. In this example with
>>> +/20ms, this is the case with packet rate larger than 25pps.
>>> Reordering has bad effects on transport protocol throughputs.
>>> Reordering is less common on the Internet, so I don't want to emulate
>>> such. I don't say there is no reordering, I just say I don't want this
>>> netem behavior.
>>>
>>> The netem guide mentions this unexpected results. It was caused by a
>>> change in version 1.1 (2.6.15). For people like me that do not want
>>> this, there is a work-around mentioned. However, this doesn't work
>>> anymore since somewhere before 2.6.31.
>>> http://www.linuxfoundation.org/collaborate/workgroups/networking/netem
>>> https://lists.linux-foundation.org/pipermail/netem/2011-April/001507.html
>>>
>>> So what to do? The recent patch from Eric Dumazet eliminates
>>> jitter_reordering when netem rate is configured. Maybe not the clean
>>> approach, but acceptable. Ferry his patch eliminates
>>> jitter_reordering. Some people may use reorder by jitter, so this
>>> "feature" should not get removed. So Ferry his patch is not accepted.
>>> On the other hand, people like me are very confused by current
>>> behavior.
>>>
>>> What we could do is keep existing features and describe what netem
>>> currently does. That is:
>>> - netem with delay and jitter may reorder packets, if inter packet
>>> spacing is smaller than jitter
>>> - reordering caused by delay and jitter can be turned off by using
>>> netem rate. rate can be set to very high value is no shaping is
>>> wanted.
>>
>> As long as one can define the expected behavior, you can add whatever
>> new netem parameter.
>>
>> One could envision adding flow separation (skb->sk or rxhashing), so
>> that each flow can have his own local queue, to guarantee no reorders
>> per flow _if_ this is needed, even if per flow delays/jitter is/are
>> configured.
>>
>> We also use netem to test on large scale, where the reordering stuff
>> needs fixes in transport stacks (And yes, we are working on TCP stack
>> to permit higher levels of reorders)
>
> I am happy with any solution that is:
> * allows both always ordering and reordering based on random jitter of delay
> * documented
>
> I do get worried that people's tests get different results because of
> netem behavior changes. Researchers like to have repeatable results.
Questions that pops out of of my head: What is more important, understandable configuration or keep existing behavior for unmodified configuration? Do we want to reorder, even with configured rate?
My preference: make in understandable, document well and make all sensible options available.
Teco
^ permalink raw reply
* Re: [PATCH RESEND net-next 06/16] sfc: Introduce and use MCDI_DECLARE_BUF macro
From: Ben Hutchings @ 2013-08-23 15:52 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1377253570.2586.15.camel@deadeye.wl.decadent.org.uk>
On Fri, 2013-08-23 at 11:26 +0100, Ben Hutchings wrote:
> On Thu, 2013-08-22 at 22:02 -0700, David Miller wrote:
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Thu, 22 Aug 2013 20:26:49 +0100
> >
> > > MCDI_DECLARE_BUF declares a variable as an MCDI buffer of the
> > > requested length, adding any necessary padding.
> > >
> > > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> >
> > There really isn't a guarentee that a u8 array is going to
> > aligned appropriately to write dwords there using casts like
> > you do.
>
> Yes, but this has been wrong for years.
>
> > Since you're here macro'izing the declaration, take the time to fix it
> > to specify the necessary alignment as well.
> [...]
>
> My intent was to separate refactoring changes with no immediate effect
> (#6-9) from functional changes (#10-11), making them easier to review.
> The alignment gets fixed by #11.
Does this explanation resolve your concerns or do you still want me to
squash a fix into this commit?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: netem: the reorder discussion
From: Stephen Hemminger @ 2013-08-23 15:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: Teco Boot, Ferry Huberts, Netem, netdev@vger.kernel.org,
core-users@pf.itd.nrl.navy.mil
In-Reply-To: <1377270756.8828.5.camel@edumazet-glaptop>
On Fri, 23 Aug 2013 08:12:36 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2013-08-23 at 16:16 +0200, Teco Boot wrote:
> > I had reorder problems with netem qdisc. I make use of the NRL CORE
> > network emulator, which sets up virtual routers and links, build with
> > network namespaces and netem. Typical CORE config for a n1<-->n2 link
> > with rate (1024Kbps), delay (50ms) and no jitter:
> > qdisc tbf 1: dev n1.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
> > lat 488.6ms
> > qdisc netem 10: dev n1.eth0.222 parent 1:1 limit 1000 delay 50.0ms
> > qdisc tbf 1: dev n2.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
> > lat 488.6ms
> > qdisc netem 10: dev n2.eth0.222 parent 1:1 limit 1000 delay 50.0ms
> >
> > I added jitter to this bi-directional link, e.g. 20ms. Now the delay
> > for each packet is 50ms +/- 20ms is 30ms to 70ms.
> > However, this has some unexpected results: packets may be reordered.
> > That is because the actual delay is calculated for each packet. Some
> > packets have a larger delay, some have a smaller. If the inter-packet
> > spacing is smaller than the time differences set by netem (up to 2x
> > configured jitter), packets are reordered. In this example with
> > +/20ms, this is the case with packet rate larger than 25pps.
> > Reordering has bad effects on transport protocol throughputs.
> > Reordering is less common on the Internet, so I don't want to emulate
> > such. I don't say there is no reordering, I just say I don't want this
> > netem behavior.
> >
> > The netem guide mentions this unexpected results. It was caused by a
> > change in version 1.1 (2.6.15). For people like me that do not want
> > this, there is a work-around mentioned. However, this doesn't work
> > anymore since somewhere before 2.6.31.
> > http://www.linuxfoundation.org/collaborate/workgroups/networking/netem
> > https://lists.linux-foundation.org/pipermail/netem/2011-April/001507.html
> >
> > So what to do? The recent patch from Eric Dumazet eliminates
> > jitter_reordering when netem rate is configured. Maybe not the clean
> > approach, but acceptable. Ferry his patch eliminates
> > jitter_reordering. Some people may use reorder by jitter, so this
> > "feature" should not get removed. So Ferry his patch is not accepted.
> > On the other hand, people like me are very confused by current
> > behavior.
> >
> > What we could do is keep existing features and describe what netem
> > currently does. That is:
> > - netem with delay and jitter may reorder packets, if inter packet
> > spacing is smaller than jitter
> > - reordering caused by delay and jitter can be turned off by using
> > netem rate. rate can be set to very high value is no shaping is
> > wanted.
>
> As long as one can define the expected behavior, you can add whatever
> new netem parameter.
>
> One could envision adding flow separation (skb->sk or rxhashing), so
> that each flow can have his own local queue, to guarantee no reorders
> per flow _if_ this is needed, even if per flow delays/jitter is/are
> configured.
>
> We also use netem to test on large scale, where the reordering stuff
> needs fixes in transport stacks (And yes, we are working on TCP stack
> to permit higher levels of reorders)
I am happy with any solution that is:
* allows both always ordering and reordering based on random jitter of delay
* documented
I do get worried that people's tests get different results because of
netem behavior changes. Researchers like to have repeatable results.
^ permalink raw reply
* Re: [PATCH net-next] net: tcp_probe: allow more advanced ingress filtering by mark
From: Daniel Borkmann @ 2013-08-23 15:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev
In-Reply-To: <1377270813.8828.6.camel@edumazet-glaptop>
On 08/23/2013 05:13 PM, Eric Dumazet wrote:
> On Fri, 2013-08-23 at 16:16 +0200, Daniel Borkmann wrote:
>> Currently, the tcp_probe snooper can either filter packets by a given
>> port (handed to the module via module parameter e.g. port=80) or lets
>> all TCP traffic pass (port=0, default). When a port is specified, the
>> port number is tested against the sk's source/destination port. Thus,
>> if one of them matches, the information will be further processed for
>> the log.
>>
>> As this is quite limited, allow for more advanced filtering possibilities
>> which can facilitate debugging/analysis with the help of the tcp_probe
>> snooper. Therefore, similarly as added to BPF machine in commit 7e75f93e
>> ("pkt_sched: ingress socket filter by mark"), add the possibility to
>> use skb->mark as a filter.
>>
>> If the mark is not being used otherwise, this allows ingress filtering
>> by flow (e.g. in order to track updates from only a single flow, or a
>> subset of all flows for a given port) and other things such as dynamic
>> logging and reconfiguration without removing/re-inserting the tcp_probe
>> module, etc. Simple example:
>>
>> insmod net/ipv4/tcp_probe.ko fwmark=8888 full=1
>> ...
>> iptables -A INPUT -i eth4 -t mangle -p tcp --dport 22 \
>> --sport 60952 -j MARK --set-mark 8888
>> [... sampling interval ...]
>> iptables -D INPUT -i eth4 -t mangle -p tcp --dport 22 \
>> --sport 60952 -j MARK --set-mark 8888
>>
>> The current option to filter by a given port is still being preserved. A
>> similar approach could be done for the sctp_probe module as a follow-up.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> ---
>
> Well, why not adding ability to load/use a BPF filter instead ? ;)
Hehe. ;-)
I think this patch is minimal and least intrusive and for observing flows
the skb->mark would provide an easy job for us of quickly configuring and
altering a filter during runtime through iptables or other tools.
If we would tell the user to load some BPF bytecode first through tool <xyz>,
we would need an additional interface of injecting this into tcp_probe, and
the ability to atomically change a filter at run-time. Likely, that the
biggest use-case would be to filter particular flows in order to observe
TCP params, I think the whole BPF machinery would be sort of overkill for
that, although you know I like playing with BPF. ;-)
So I think these 15 lines of code from this patch are less intrusive and
more clean than the alternative.
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Benoit Cousson @ 2013-08-23 15:22 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52177052.1030308@ti.com>
Hi Santosh,
[...]
>> +static const struct of_device_id cpsw_of_mtable[] = {
>> + {
>> + .compatible = "ti,am3352-cpsw",
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.
Hehe, welcome to the club!
I keep arguing about that as well :-)
I think we should create a Facebook group:
"against-the-soc-version-in-the-compatible-string".
Regards,
Benoit
^ permalink raw reply
* Re: [PATCH net-next] net: tcp_probe: allow more advanced ingress filtering by mark
From: Eric Dumazet @ 2013-08-23 15:13 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1377267393-24604-1-git-send-email-dborkman@redhat.com>
On Fri, 2013-08-23 at 16:16 +0200, Daniel Borkmann wrote:
> Currently, the tcp_probe snooper can either filter packets by a given
> port (handed to the module via module parameter e.g. port=80) or lets
> all TCP traffic pass (port=0, default). When a port is specified, the
> port number is tested against the sk's source/destination port. Thus,
> if one of them matches, the information will be further processed for
> the log.
>
> As this is quite limited, allow for more advanced filtering possibilities
> which can facilitate debugging/analysis with the help of the tcp_probe
> snooper. Therefore, similarly as added to BPF machine in commit 7e75f93e
> ("pkt_sched: ingress socket filter by mark"), add the possibility to
> use skb->mark as a filter.
>
> If the mark is not being used otherwise, this allows ingress filtering
> by flow (e.g. in order to track updates from only a single flow, or a
> subset of all flows for a given port) and other things such as dynamic
> logging and reconfiguration without removing/re-inserting the tcp_probe
> module, etc. Simple example:
>
> insmod net/ipv4/tcp_probe.ko fwmark=8888 full=1
> ...
> iptables -A INPUT -i eth4 -t mangle -p tcp --dport 22 \
> --sport 60952 -j MARK --set-mark 8888
> [... sampling interval ...]
> iptables -D INPUT -i eth4 -t mangle -p tcp --dport 22 \
> --sport 60952 -j MARK --set-mark 8888
>
> The current option to filter by a given port is still being preserved. A
> similar approach could be done for the sctp_probe module as a follow-up.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
Well, why not adding ability to load/use a BPF filter instead ? ;)
^ 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