* Re: [net-next 13/13] igb: Store the MAC address in the name in the PTP struct.
From: Ben Hutchings @ 2012-08-23 21:35 UTC (permalink / raw)
To: Richard Cochran
Cc: Jeff Kirsher, davem, Matthew Vick, netdev, gospo, sassmann,
Stuart Hodgson
In-Reply-To: <20120823104554.GA2238@netboy.at.omicron.at>
On Thu, 2012-08-23 at 12:45 +0200, Richard Cochran wrote:
> On Thu, Aug 23, 2012 at 02:56:53AM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> >
> > Change the name of the adapter in the PTP struct to enable easier
> > correlation between interface and PTP device.
>
> You want to put the MAC address into the clock name? This is wrong.
>
> Besides, there is no need for this. The ethool method already makes
> the correlation crystal clear.
The name field is described as 'A short name to identify the clock'. It
is not stated whether this is meant to be the name of the clock *device*
or the clock *driver*. If it's the name of the device then some unique
ID such as the permanent MAC address is required.
The ixgbe driver uses MAC addresses and so did the last submitted
version of PHC support for the sfc driver.
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
* Newsletter.
From: Zimbra. @ 2012-08-23 21:21 UTC (permalink / raw)
This mail is from Zimbra Administrator we wish to bring to your notice the Condition of your email account.
We have just noticed that you have exceeded your email Database limit of 500 MB quota and your email IP is causing conflict because it is been accessed in different server location. You need to Upgrade and expand your email quota limit before you can continue to use your email.
Update your email quota limit to 2.6 GB, use the below web link:
https://docs.google.com/spreadsheet/viewform?formkey=dFRGbU1iYU0tdm1qeWlCSDhLZjZVOFE6MQ
Failure to do this will result to email deactivation within 24hours
Thank you for your understanding.
Copyright 2012 © Inc. Zimbra Help Desk.
^ permalink raw reply
* Newsletter.
From: Zimbra. @ 2012-08-23 21:21 UTC (permalink / raw)
This mail is from Zimbra Administrator we wish to bring to your notice the Condition of your email account.
We have just noticed that you have exceeded your email Database limit of 500 MB quota and your email IP is causing conflict because it is been accessed in different server location. You need to Upgrade and expand your email quota limit before you can continue to use your email.
Update your email quota limit to 2.6 GB, use the below web link:
https://docs.google.com/spreadsheet/viewform?formkey=dFRGbU1iYU0tdm1qeWlCSDhLZjZVOFE6MQ
Failure to do this will result to email deactivation within 24hours
Thank you for your understanding.
Copyright 2012 © Inc. Zimbra Help Desk.
^ permalink raw reply
* [PATCH] net/fsl_pq_mdio: add support for the Fman 1G MDIO controller
From: Timur Tabi @ 2012-08-23 21:24 UTC (permalink / raw)
To: Andy Fleming, David Miller, netdev
The MDIO controller on the Frame Manager (Fman) is compatible with the
QE and Gianfar MDIO controllers, but we don't care about the TBI because
the Ethernet drivers (FMD) take care of programming it.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/net/ethernet/freescale/fsl_pq_mdio.c | 36 ++++++++++++++++----------
1 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index 9527b28..3af87cb 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -264,7 +264,7 @@ static int fsl_pq_mdio_probe(struct platform_device *ofdev)
struct fsl_pq_mdio_priv *priv;
struct fsl_pq_mdio __iomem *regs = NULL;
void __iomem *map;
- u32 __iomem *tbipa;
+ u32 __iomem *tbipa = NULL;
struct mii_bus *new_bus;
int tbiaddr = -1;
const u32 *addrp;
@@ -310,6 +310,7 @@ static int fsl_pq_mdio_probe(struct platform_device *ofdev)
if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
of_device_is_compatible(np, "fsl,gianfar-tbi") ||
+ of_device_is_compatible(np, "fsl,fman-mdio") ||
of_device_is_compatible(np, "fsl,ucc-mdio") ||
of_device_is_compatible(np, "ucc_geth_phy"))
map -= offsetof(struct fsl_pq_mdio, miimcfg);
@@ -350,27 +351,31 @@ static int fsl_pq_mdio_probe(struct platform_device *ofdev)
mii_mng_master = id;
ucc_set_qe_mux_mii_mng(id - 1);
}
+ } else if (of_device_is_compatible(np, "fsl,fman-mdio")) {
+ /* No TBI operations needed for Fman, but don't fail either */
} else {
err = -ENODEV;
goto err_free_irqs;
}
- for_each_child_of_node(np, tbi) {
- if (!strncmp(tbi->type, "tbi-phy", 8))
- break;
- }
+ if (tbipa) {
+ for_each_child_of_node(np, tbi) {
+ if (!strncmp(tbi->type, "tbi-phy", 8))
+ break;
+ }
- if (tbi) {
- const u32 *prop = of_get_property(tbi, "reg", NULL);
+ if (tbi) {
+ const u32 *prop = of_get_property(tbi, "reg", NULL);
- if (prop)
- tbiaddr = *prop;
+ if (prop)
+ tbiaddr = be32_to_cpup(prop);
- if (tbiaddr == -1) {
- err = -EBUSY;
- goto err_free_irqs;
- } else {
- out_be32(tbipa, tbiaddr);
+ if (tbiaddr == -1) {
+ err = -EBUSY;
+ goto err_free_irqs;
+ } else {
+ out_be32(tbipa, tbiaddr);
+ }
}
}
@@ -437,6 +442,9 @@ static struct of_device_id fsl_pq_mdio_match[] = {
{
.compatible = "fsl,etsec2-mdio",
},
+ {
+ .compatible = "fsl,fman-mdio",
+ },
{},
};
MODULE_DEVICE_TABLE(of, fsl_pq_mdio_match);
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] ipw2200: use is_zero_ether_addr() and is_broadcast_ether_addr()
From: Stanislav Yakovlev @ 2012-08-23 21:13 UTC (permalink / raw)
To: Wei Yongjun; +Cc: linville, yongjun_wei, linux-wireless, netdev
In-Reply-To: <CAPgLHd_K=bbQ=zpKY8kW3HCGeuE3GwCCQOAg+gJoFdCv9SnVrg@mail.gmail.com>
Hi,
On 23 August 2012 10:54, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Using is_zero_ether_addr() and is_broadcast_ether_addr() instead of
> directly use memcmp() to determine if the ethernet address is all zeros.
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/net/wireless/ipw2x00/ipw2200.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
Looks fine, thanks.
Stanislav.
^ permalink raw reply
* Re: [RFC PATCH bridge 0/5] Add basic VLAN support to bridges
From: Stephen Hemminger @ 2012-08-23 21:12 UTC (permalink / raw)
To: Nicolas de Pesloüan; +Cc: Vlad Yasevich, netdev
In-Reply-To: <50369A99.2070405@gmail.com>
On Thu, 23 Aug 2012 23:03:21 +0200
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:
> Le 23/08/2012 21:29, Vlad Yasevich a écrit :
> > This series of patches provides an ability to add VLAN IDs to the bridge
> > ports. This is similar to what can be found in most switches. The bridge
> > port may have any number of VLANs added to it including vlan 0 for untagged
> > traffic. When vlans are added to the port, only traffic tagged with particular
> > vlan will forwarded over this port. Additionally, vlan ids are added to FDB
> > entries and become part of the lookup. This way we correctly identify the FDB
> > entry.
> >
> > There are still pieces missing. I don't yet support adding a static fdb entry
> > with a particular vlan. There is no netlink support for carrying a vlan id.
> >
> > I'd like to hear thoughts of whether this is usufull and something we should
> > persue.
> >
>
> Do you think this might allow for per VLAN spanning tree (having ports in forwarding state or
> blocking state depending on the VLAN) in the future?
>
> Nicolas.
Completely different problem
^ permalink raw reply
* Re: [PATCH] ipw2100: use is_zero_ether_addr() and is_broadcast_ether_addr()
From: Stanislav Yakovlev @ 2012-08-23 21:12 UTC (permalink / raw)
To: Wei Yongjun
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAPgLHd_zKkfbms8pMPiTKtGxWXw9=b0+AftqFxWZCPLb3EBaig-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 23 August 2012 10:53, Wei Yongjun <weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>
> Using is_zero_ether_addr() and is_broadcast_ether_addr() instead of
> directly use memcmp() to determine if the ethernet address is all zeros.
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
> ---
> drivers/net/wireless/ipw2x00/ipw2100.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
Looks fine, thanks.
Stanislav.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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
* RE: [PATCH v2] net: add new QCA alx ethernet driver
From: Ben Hutchings @ 2012-08-23 21:09 UTC (permalink / raw)
To: Huang, Xiong
Cc: David Miller, Ren, Cloud, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Rodriguez, Luis
In-Reply-To: <157393863283F442885425D2C45428562A4F5169@nasanexd02f.na.qualcomm.com>
On Thu, 2012-08-23 at 06:35 +0000, Huang, Xiong wrote:
> > This is why we require that portable, sane, interfaces are added to ethtool for
> > driver diagnostics. That way users can perform a task in the same way
> > regardless of what hardware and driver are underneath.
>
> I quite agree you on using ethtool to implement it. we did consider it.
> But ethtool has some limitation, for example, the NIC has built-in OTP (TWSI interface)
> And Flash (External SPI interface), their properties are quite different with EEPROM which
> Ethtool supports.
> To support such memory (OTP/Flash), we need additional input parameters.
You have two reasonable options for this:
1. The ETHTOOL_FLASHDEV command takes a partition ID and filename to
write. The driver is supposed to load the file through the firmware
loader and then rewrite the partition completely (erasing if necessary).
Example: be2net.
2. For a more flexible interface, implement an MTD driver as part of
your net driver. Example: sfc.
> Same situation exists in diagnostic utility. Ethtool only provide two options : offline & online
> That's too gross to locate which part/module of the chip is malfunction. we also need
> more options to detect it.
That's absolute nonsense, you can run as many sub-tests as you want and
provide separate results for each of them.
Ben.
> that's why we finally selected a custom debugfs interface.
--
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: [RFC PATCH bridge 0/5] Add basic VLAN support to bridges
From: Nicolas de Pesloüan @ 2012-08-23 21:03 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, Stephen Hemminger
In-Reply-To: <1345750195-31598-1-git-send-email-vyasevic@redhat.com>
Le 23/08/2012 21:29, Vlad Yasevich a écrit :
> This series of patches provides an ability to add VLAN IDs to the bridge
> ports. This is similar to what can be found in most switches. The bridge
> port may have any number of VLANs added to it including vlan 0 for untagged
> traffic. When vlans are added to the port, only traffic tagged with particular
> vlan will forwarded over this port. Additionally, vlan ids are added to FDB
> entries and become part of the lookup. This way we correctly identify the FDB
> entry.
>
> There are still pieces missing. I don't yet support adding a static fdb entry
> with a particular vlan. There is no netlink support for carrying a vlan id.
>
> I'd like to hear thoughts of whether this is usufull and something we should
> persue.
>
Do you think this might allow for per VLAN spanning tree (having ports in forwarding state or
blocking state depending on the VLAN) in the future?
Nicolas.
^ permalink raw reply
* Re: [RFC PATCH bridge 1/5] bridge: Add vlan check to forwarding path
From: Nicolas de Pesloüan @ 2012-08-23 20:58 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1345750195-31598-2-git-send-email-vyasevic@redhat.com>
Le 23/08/2012 21:29, Vlad Yasevich a écrit :
> -/* Don't forward packets to originating port or forwarding diasabled */
> +/* Don't forward packets to originating port or forwarding diasabled.
While you work on this, feel free to fix the typo in disabled ^^^^^^^^^
Nicolas.
^ permalink raw reply
* Re: [PATCH 01/14] aoe: for performance support larger packet payloads
From: Ed Cashin @ 2012-08-23 20:46 UTC (permalink / raw)
To: netdev; +Cc: Ed Cashin
In-Reply-To: <7c895879cfd1e15dd76c2469b417b48d0462d7df.1345743801.git.ecashin@coraid.com>
I should have Cc'ed netdev for this patch. The series was only sent to linux-kernel, sorry.
On Aug 17, 2012, at 9:18 PM, Ed Cashin wrote:
> This patch adds the ability to work with large packets composed of a
> number of segments, using the scatter gather feature of the block
> layer (biovecs) and the network layer (skb frag array). The
> motivation is the performance gained by using a packet data payload
> greater than a page size and by using the network card's scatter
> gather feature.
>
> Users of the out-of-tree aoe driver already had these changes, but
> since early 2011, they have complained of increased memory utilization
> and higher CPU utilization during heavy writes.[1] The commit below
> appears related, as it disables scatter gather on non-IP protocols
> inside the harmonize_features function, even when the NIC supports sg.
>
> commit f01a5236bd4b140198fbcc550f085e8361fd73fa
> Author: Jesse Gross <jesse@nicira.com>
> Date: Sun Jan 9 06:23:31 2011 +0000
>
> net offloading: Generalize netif_get_vlan_features().
>
> With that regression in place, transmits always linearize sg AoE
> packets, but in-kernel users did not have this patch. Before 2.6.38,
> though, these changes were working to allow sg to increase
> performance.
>
> 1. http://www.spinics.net/lists/linux-mm/msg15184.html
>
> Signed-off-by: Ed Cashin <ecashin@coraid.com>
> ---
> drivers/block/aoe/aoe.h | 2 +
> drivers/block/aoe/aoeblk.c | 3 +
> drivers/block/aoe/aoecmd.c | 138 ++++++++++++++++++++++++++++++-------------
> drivers/block/aoe/aoedev.c | 1 +
> drivers/block/aoe/aoenet.c | 13 +++-
> 5 files changed, 111 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
> index db195ab..8ca8c8a 100644
> --- a/drivers/block/aoe/aoe.h
> +++ b/drivers/block/aoe/aoe.h
> @@ -119,6 +119,8 @@ struct frame {
> ulong bcnt;
> sector_t lba;
> struct sk_buff *skb;
> + struct bio_vec *bv;
> + ulong bv_off;
> };
>
> struct aoeif {
> diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
> index 321de7b..1471f81 100644
> --- a/drivers/block/aoe/aoeblk.c
> +++ b/drivers/block/aoe/aoeblk.c
> @@ -279,6 +279,9 @@ aoeblk_gdalloc(void *vp)
> if (bdi_init(&d->blkq->backing_dev_info))
> goto err_blkq;
> spin_lock_irqsave(&d->lock, flags);
> + blk_queue_max_hw_sectors(d->blkq, BLK_DEF_MAX_SECTORS);
> + d->blkq->backing_dev_info.ra_pages = BLK_DEF_MAX_SECTORS * 1024;
> + d->blkq->backing_dev_info.ra_pages /= PAGE_CACHE_SIZE;
> gd->major = AOE_MAJOR;
> gd->first_minor = d->sysminor * AOE_PARTITIONS;
> gd->fops = &aoe_bdops;
> diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
> index de0435e..f10ab49 100644
> --- a/drivers/block/aoe/aoecmd.c
> +++ b/drivers/block/aoe/aoecmd.c
> @@ -164,7 +164,8 @@ freeframe(struct aoedev *d)
> rf = f;
> continue;
> }
> -gotone: skb_shinfo(skb)->nr_frags = skb->data_len = 0;
> +gotone: skb->truesize -= skb->data_len;
> + skb_shinfo(skb)->nr_frags = skb->data_len = 0;
> skb_trim(skb, 0);
> d->tgt = t;
> ifrotate(*t);
> @@ -200,6 +201,24 @@ gotone: skb_shinfo(skb)->nr_frags = skb->data_len = 0;
> return NULL;
> }
>
> +static void
> +skb_fillup(struct sk_buff *skb, struct bio_vec *bv, ulong off, ulong cnt)
> +{
> + int frag = 0;
> + ulong fcnt;
> +loop:
> + fcnt = bv->bv_len - (off - bv->bv_offset);
> + if (fcnt > cnt)
> + fcnt = cnt;
> + skb_fill_page_desc(skb, frag++, bv->bv_page, off, fcnt);
> + cnt -= fcnt;
> + if (cnt <= 0)
> + return;
> + bv++;
> + off = bv->bv_offset;
> + goto loop;
> +}
> +
> static int
> aoecmd_ata_rw(struct aoedev *d)
> {
> @@ -210,7 +229,7 @@ aoecmd_ata_rw(struct aoedev *d)
> struct bio_vec *bv;
> struct aoetgt *t;
> struct sk_buff *skb;
> - ulong bcnt;
> + ulong bcnt, fbcnt;
> char writebit, extbit;
>
> writebit = 0x10;
> @@ -225,8 +244,28 @@ aoecmd_ata_rw(struct aoedev *d)
> bcnt = t->ifp->maxbcnt;
> if (bcnt == 0)
> bcnt = DEFAULTBCNT;
> - if (bcnt > buf->bv_resid)
> - bcnt = buf->bv_resid;
> + if (bcnt > buf->resid)
> + bcnt = buf->resid;
> + fbcnt = bcnt;
> + f->bv = buf->bv;
> + f->bv_off = f->bv->bv_offset + (f->bv->bv_len - buf->bv_resid);
> + do {
> + if (fbcnt < buf->bv_resid) {
> + buf->bv_resid -= fbcnt;
> + buf->resid -= fbcnt;
> + break;
> + }
> + fbcnt -= buf->bv_resid;
> + buf->resid -= buf->bv_resid;
> + if (buf->resid == 0) {
> + d->inprocess = NULL;
> + break;
> + }
> + buf->bv++;
> + buf->bv_resid = buf->bv->bv_len;
> + WARN_ON(buf->bv_resid == 0);
> + } while (fbcnt);
> +
> /* initialize the headers & frame */
> skb = f->skb;
> h = (struct aoe_hdr *) skb_mac_header(skb);
> @@ -237,7 +276,6 @@ aoecmd_ata_rw(struct aoedev *d)
> t->nout++;
> f->waited = 0;
> f->buf = buf;
> - f->bufaddr = page_address(bv->bv_page) + buf->bv_off;
> f->bcnt = bcnt;
> f->lba = buf->sector;
>
> @@ -252,10 +290,11 @@ aoecmd_ata_rw(struct aoedev *d)
> ah->lba3 |= 0xe0; /* LBA bit + obsolete 0xa0 */
> }
> if (bio_data_dir(buf->bio) == WRITE) {
> - skb_fill_page_desc(skb, 0, bv->bv_page, buf->bv_off, bcnt);
> + skb_fillup(skb, f->bv, f->bv_off, bcnt);
> ah->aflags |= AOEAFL_WRITE;
> skb->len += bcnt;
> skb->data_len = bcnt;
> + skb->truesize += bcnt;
> t->wpkts++;
> } else {
> t->rpkts++;
> @@ -266,18 +305,7 @@ aoecmd_ata_rw(struct aoedev *d)
>
> /* mark all tracking fields and load out */
> buf->nframesout += 1;
> - buf->bv_off += bcnt;
> - buf->bv_resid -= bcnt;
> - buf->resid -= bcnt;
> buf->sector += bcnt >> 9;
> - if (buf->resid == 0) {
> - d->inprocess = NULL;
> - } else if (buf->bv_resid == 0) {
> - buf->bv = ++bv;
> - buf->bv_resid = bv->bv_len;
> - WARN_ON(buf->bv_resid == 0);
> - buf->bv_off = bv->bv_offset;
> - }
>
> skb->dev = t->ifp->nd;
> skb = skb_clone(skb, GFP_ATOMIC);
> @@ -364,14 +392,12 @@ resend(struct aoedev *d, struct aoetgt *t, struct frame *f)
> put_lba(ah, f->lba);
>
> n = f->bcnt;
> - if (n > DEFAULTBCNT)
> - n = DEFAULTBCNT;
> ah->scnt = n >> 9;
> if (ah->aflags & AOEAFL_WRITE) {
> - skb_fill_page_desc(skb, 0, virt_to_page(f->bufaddr),
> - offset_in_page(f->bufaddr), n);
> + skb_fillup(skb, f->bv, f->bv_off, n);
> skb->len = sizeof *h + sizeof *ah + n;
> skb->data_len = n;
> + skb->truesize += n;
> }
> }
> skb->dev = t->ifp->nd;
> @@ -530,20 +556,6 @@ rexmit_timer(ulong vp)
> ejectif(t, ifp);
> ifp = NULL;
> }
> -
> - if (ata_scnt(skb_mac_header(f->skb)) > DEFAULTBCNT / 512
> - && ifp && ++ifp->lostjumbo > (t->nframes << 1)
> - && ifp->maxbcnt != DEFAULTBCNT) {
> - printk(KERN_INFO
> - "aoe: e%ld.%d: "
> - "too many lost jumbo on "
> - "%s:%pm - "
> - "falling back to %d frames.\n",
> - d->aoemajor, d->aoeminor,
> - ifp->nd->name, t->addr,
> - DEFAULTBCNT);
> - ifp->maxbcnt = 0;
> - }
> resend(d, t, f);
> }
>
> @@ -736,6 +748,45 @@ diskstats(struct gendisk *disk, struct bio *bio, ulong duration, sector_t sector
> part_stat_unlock();
> }
>
> +static void
> +bvcpy(struct bio_vec *bv, ulong off, struct sk_buff *skb, ulong cnt)
> +{
> + ulong fcnt;
> + char *p;
> + int soff = 0;
> +loop:
> + fcnt = bv->bv_len - (off - bv->bv_offset);
> + if (fcnt > cnt)
> + fcnt = cnt;
> + p = page_address(bv->bv_page) + off;
> + skb_copy_bits(skb, soff, p, fcnt);
> + soff += fcnt;
> + cnt -= fcnt;
> + if (cnt <= 0)
> + return;
> + bv++;
> + off = bv->bv_offset;
> + goto loop;
> +}
> +
> +static void
> +fadvance(struct frame *f, ulong cnt)
> +{
> + ulong fcnt;
> +
> + f->lba += cnt >> 9;
> +loop:
> + fcnt = f->bv->bv_len - (f->bv_off - f->bv->bv_offset);
> + if (fcnt > cnt) {
> + f->bv_off += cnt;
> + return;
> + }
> + cnt -= fcnt;
> + f->bv++;
> + f->bv_off = f->bv->bv_offset;
> + goto loop;
> +}
> +
> void
> aoecmd_ata_rsp(struct sk_buff *skb)
> {
> @@ -753,6 +804,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
> u16 aoemajor;
>
> hin = (struct aoe_hdr *) skb_mac_header(skb);
> + skb_pull(skb, sizeof(*hin));
> aoemajor = get_unaligned_be16(&hin->major);
> d = aoedev_by_aoeaddr(aoemajor, hin->minor);
> if (d == NULL) {
> @@ -790,7 +842,8 @@ aoecmd_ata_rsp(struct sk_buff *skb)
>
> calc_rttavg(d, tsince(f->tag));
>
> - ahin = (struct aoe_atahdr *) (hin+1);
> + ahin = (struct aoe_atahdr *) skb->data;
> + skb_pull(skb, sizeof(*ahin));
> hout = (struct aoe_hdr *) skb_mac_header(f->skb);
> ahout = (struct aoe_atahdr *) (hout+1);
> buf = f->buf;
> @@ -809,7 +862,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
> switch (ahout->cmdstat) {
> case ATA_CMD_PIO_READ:
> case ATA_CMD_PIO_READ_EXT:
> - if (skb->len - sizeof *hin - sizeof *ahin < n) {
> + if (skb->len < n) {
> printk(KERN_ERR
> "aoe: %s. skb->len=%d need=%ld\n",
> "runt data size in read", skb->len, n);
> @@ -817,7 +870,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
> spin_unlock_irqrestore(&d->lock, flags);
> return;
> }
> - memcpy(f->bufaddr, ahin+1, n);
> + bvcpy(f->bv, f->bv_off, skb, n);
> case ATA_CMD_PIO_WRITE:
> case ATA_CMD_PIO_WRITE_EXT:
> ifp = getif(t, skb->dev);
> @@ -827,21 +880,22 @@ aoecmd_ata_rsp(struct sk_buff *skb)
> ifp->lostjumbo = 0;
> }
> if (f->bcnt -= n) {
> - f->lba += n >> 9;
> - f->bufaddr += n;
> + fadvance(f, n);
> resend(d, t, f);
> goto xmit;
> }
> break;
> case ATA_CMD_ID_ATA:
> - if (skb->len - sizeof *hin - sizeof *ahin < 512) {
> + if (skb->len < 512) {
> printk(KERN_INFO
> "aoe: runt data size in ataid. skb->len=%d\n",
> skb->len);
> spin_unlock_irqrestore(&d->lock, flags);
> return;
> }
> - ataid_complete(d, t, (char *) (ahin+1));
> + if (skb_linearize(skb))
> + break;
> + ataid_complete(d, t, skb->data);
> break;
> default:
> printk(KERN_INFO
> diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
> index 6b5110a..b2d1fd3 100644
> --- a/drivers/block/aoe/aoedev.c
> +++ b/drivers/block/aoe/aoedev.c
> @@ -182,6 +182,7 @@ skbfree(struct sk_buff *skb)
> "cannot free skb -- memory leaked.");
> return;
> }
> + skb->truesize -= skb->data_len;
> skb_shinfo(skb)->nr_frags = skb->data_len = 0;
> skb_trim(skb, 0);
> dev_kfree_skb(skb);
> diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
> index 4d3bc0d..0787807 100644
> --- a/drivers/block/aoe/aoenet.c
> +++ b/drivers/block/aoe/aoenet.c
> @@ -102,7 +102,9 @@ static int
> aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt, struct net_device *orig_dev)
> {
> struct aoe_hdr *h;
> + struct aoe_atahdr *ah;
> u32 n;
> + int sn;
>
> if (dev_net(ifp) != &init_net)
> goto exit;
> @@ -110,13 +112,16 @@ aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt,
> skb = skb_share_check(skb, GFP_ATOMIC);
> if (skb == NULL)
> return 0;
> - if (skb_linearize(skb))
> - goto exit;
> if (!is_aoe_netif(ifp))
> goto exit;
> skb_push(skb, ETH_HLEN); /* (1) */
> -
> - h = (struct aoe_hdr *) skb_mac_header(skb);
> + sn = sizeof(*h) + sizeof(*ah);
> + if (skb->len >= sn) {
> + sn -= skb_headlen(skb);
> + if (sn > 0 && !__pskb_pull_tail(skb, sn))
> + goto exit;
> + }
> + h = (struct aoe_hdr *) skb->data;
> n = get_unaligned_be32(&h->tag);
> if ((h->verfl & AOEFL_RSP) == 0 || (n & 1<<31))
> goto exit;
> --
> 1.7.2.5
>
--
Ed Cashin
ecashin@coraid.com
^ permalink raw reply
* [PATCH] iproute2: Add FDB print and update cmds for self and master
From: John Fastabend @ 2012-08-23 20:37 UTC (permalink / raw)
To: shemminger; +Cc: netdev, vyasevic
Add command to update and print FDB entries with NTF_SELF and
NTF_MASTER set.
Example usages illustrating use of 'self' to program embedded
forwarding table and 'master' to configure the forwarding table
of the bridge. Also shows 'master self' used to update both in
the same command.
#./br/br fdb add 00:1b:21:55:23:60 dev eth3 self
#./br/br fdb add 00:1b:21:55:23:60 dev eth3 master
#./br/br fdb add 00:1b:21:55:23:61 dev eth3 self master
#./br/br fdb add 00:1b:21:55:23:62 dev eth3
#./br/br fdb show
eth3 00:1b:21:55:23:60 local self
eth3 00:1b:21:55:23:61 local self
eth3 33:33:00:00:00:01 local self
eth3 01:00:5e:00:00:01 local self
eth3 33:33:ff:55:23:59 local self
eth3 01:00:5e:00:00:fb local self
eth33 33:33:00:00:00:01 local self
eth34 33:33:00:00:00:01 local self
eth3 00:1b:21:55:23:59 local master
eth3 00:1b:21:55:23:60 static master
eth3 00:1b:21:55:23:62 static master
eth3 00:1b:21:55:23:61 static master
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
bridge/fdb.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index c6aa08e..cee0fcd 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -26,7 +26,7 @@ int filter_index;
static void usage(void)
{
- fprintf(stderr, "Usage: bridge fdb { add | del | replace } ADDR dev DEV\n");
+ fprintf(stderr, "Usage: bridge fdb { add | del | replace } ADDR dev DEV {self|master}\n");
fprintf(stderr, " bridge fdb {show} [ dev DEV ]\n");
exit(-1);
}
@@ -95,11 +95,12 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
return -1;
}
- printf("%s\t%.2x:%.2x:%.2x:%.2x:%.2x:%.2x\t%s",
+ printf("%s\t%.2x:%.2x:%.2x:%.2x:%.2x:%.2x\t%s %s",
ll_index_to_name(r->ndm_ifindex),
addr[0], addr[1], addr[2],
addr[3], addr[4], addr[5],
- state_n2a(r->ndm_state));
+ state_n2a(r->ndm_state),
+ (r->ndm_flags & NTF_SELF) ? "self" : "master");
if (show_stats && tb[NDA_CACHEINFO]) {
struct nda_cacheinfo *ci = RTA_DATA(tb[NDA_CACHEINFO]);
@@ -176,6 +177,10 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
req.ndm.ndm_state = NUD_PERMANENT;
} else if (strcmp(*argv, "temp") == 0) {
req.ndm.ndm_state = NUD_REACHABLE;
+ } else if (strcmp(*argv, "self") == 0) {
+ req.ndm.ndm_flags |= NTF_SELF;
+ } else if (strcmp(*argv, "master") == 0) {
+ req.ndm.ndm_flags |= NTF_MASTER;
} else {
if (strcmp(*argv, "to") == 0) {
NEXT_ARG();
^ permalink raw reply related
* [PATCH 01/14] aoe: for performance support larger packet payloads
From: Ed Cashin @ 2012-08-18 1:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, ecashin
In-Reply-To: <cover.1345743800.git.ecashin@coraid.com>
This patch adds the ability to work with large packets composed of a
number of segments, using the scatter gather feature of the block
layer (biovecs) and the network layer (skb frag array). The
motivation is the performance gained by using a packet data payload
greater than a page size and by using the network card's scatter
gather feature.
Users of the out-of-tree aoe driver already had these changes, but
since early 2011, they have complained of increased memory utilization
and higher CPU utilization during heavy writes.[1] The commit below
appears related, as it disables scatter gather on non-IP protocols
inside the harmonize_features function, even when the NIC supports sg.
commit f01a5236bd4b140198fbcc550f085e8361fd73fa
Author: Jesse Gross <jesse@nicira.com>
Date: Sun Jan 9 06:23:31 2011 +0000
net offloading: Generalize netif_get_vlan_features().
With that regression in place, transmits always linearize sg AoE
packets, but in-kernel users did not have this patch. Before 2.6.38,
though, these changes were working to allow sg to increase
performance.
1. http://www.spinics.net/lists/linux-mm/msg15184.html
Signed-off-by: Ed Cashin <ecashin@coraid.com>
---
drivers/block/aoe/aoe.h | 2 +
drivers/block/aoe/aoeblk.c | 3 +
drivers/block/aoe/aoecmd.c | 138 ++++++++++++++++++++++++++++++-------------
drivers/block/aoe/aoedev.c | 1 +
drivers/block/aoe/aoenet.c | 13 +++-
5 files changed, 111 insertions(+), 46 deletions(-)
diff --git a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h
index db195ab..8ca8c8a 100644
--- a/drivers/block/aoe/aoe.h
+++ b/drivers/block/aoe/aoe.h
@@ -119,6 +119,8 @@ struct frame {
ulong bcnt;
sector_t lba;
struct sk_buff *skb;
+ struct bio_vec *bv;
+ ulong bv_off;
};
struct aoeif {
diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
index 321de7b..1471f81 100644
--- a/drivers/block/aoe/aoeblk.c
+++ b/drivers/block/aoe/aoeblk.c
@@ -279,6 +279,9 @@ aoeblk_gdalloc(void *vp)
if (bdi_init(&d->blkq->backing_dev_info))
goto err_blkq;
spin_lock_irqsave(&d->lock, flags);
+ blk_queue_max_hw_sectors(d->blkq, BLK_DEF_MAX_SECTORS);
+ d->blkq->backing_dev_info.ra_pages = BLK_DEF_MAX_SECTORS * 1024;
+ d->blkq->backing_dev_info.ra_pages /= PAGE_CACHE_SIZE;
gd->major = AOE_MAJOR;
gd->first_minor = d->sysminor * AOE_PARTITIONS;
gd->fops = &aoe_bdops;
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index de0435e..f10ab49 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -164,7 +164,8 @@ freeframe(struct aoedev *d)
rf = f;
continue;
}
-gotone: skb_shinfo(skb)->nr_frags = skb->data_len = 0;
+gotone: skb->truesize -= skb->data_len;
+ skb_shinfo(skb)->nr_frags = skb->data_len = 0;
skb_trim(skb, 0);
d->tgt = t;
ifrotate(*t);
@@ -200,6 +201,24 @@ gotone: skb_shinfo(skb)->nr_frags = skb->data_len = 0;
return NULL;
}
+static void
+skb_fillup(struct sk_buff *skb, struct bio_vec *bv, ulong off, ulong cnt)
+{
+ int frag = 0;
+ ulong fcnt;
+loop:
+ fcnt = bv->bv_len - (off - bv->bv_offset);
+ if (fcnt > cnt)
+ fcnt = cnt;
+ skb_fill_page_desc(skb, frag++, bv->bv_page, off, fcnt);
+ cnt -= fcnt;
+ if (cnt <= 0)
+ return;
+ bv++;
+ off = bv->bv_offset;
+ goto loop;
+}
+
static int
aoecmd_ata_rw(struct aoedev *d)
{
@@ -210,7 +229,7 @@ aoecmd_ata_rw(struct aoedev *d)
struct bio_vec *bv;
struct aoetgt *t;
struct sk_buff *skb;
- ulong bcnt;
+ ulong bcnt, fbcnt;
char writebit, extbit;
writebit = 0x10;
@@ -225,8 +244,28 @@ aoecmd_ata_rw(struct aoedev *d)
bcnt = t->ifp->maxbcnt;
if (bcnt == 0)
bcnt = DEFAULTBCNT;
- if (bcnt > buf->bv_resid)
- bcnt = buf->bv_resid;
+ if (bcnt > buf->resid)
+ bcnt = buf->resid;
+ fbcnt = bcnt;
+ f->bv = buf->bv;
+ f->bv_off = f->bv->bv_offset + (f->bv->bv_len - buf->bv_resid);
+ do {
+ if (fbcnt < buf->bv_resid) {
+ buf->bv_resid -= fbcnt;
+ buf->resid -= fbcnt;
+ break;
+ }
+ fbcnt -= buf->bv_resid;
+ buf->resid -= buf->bv_resid;
+ if (buf->resid == 0) {
+ d->inprocess = NULL;
+ break;
+ }
+ buf->bv++;
+ buf->bv_resid = buf->bv->bv_len;
+ WARN_ON(buf->bv_resid == 0);
+ } while (fbcnt);
+
/* initialize the headers & frame */
skb = f->skb;
h = (struct aoe_hdr *) skb_mac_header(skb);
@@ -237,7 +276,6 @@ aoecmd_ata_rw(struct aoedev *d)
t->nout++;
f->waited = 0;
f->buf = buf;
- f->bufaddr = page_address(bv->bv_page) + buf->bv_off;
f->bcnt = bcnt;
f->lba = buf->sector;
@@ -252,10 +290,11 @@ aoecmd_ata_rw(struct aoedev *d)
ah->lba3 |= 0xe0; /* LBA bit + obsolete 0xa0 */
}
if (bio_data_dir(buf->bio) == WRITE) {
- skb_fill_page_desc(skb, 0, bv->bv_page, buf->bv_off, bcnt);
+ skb_fillup(skb, f->bv, f->bv_off, bcnt);
ah->aflags |= AOEAFL_WRITE;
skb->len += bcnt;
skb->data_len = bcnt;
+ skb->truesize += bcnt;
t->wpkts++;
} else {
t->rpkts++;
@@ -266,18 +305,7 @@ aoecmd_ata_rw(struct aoedev *d)
/* mark all tracking fields and load out */
buf->nframesout += 1;
- buf->bv_off += bcnt;
- buf->bv_resid -= bcnt;
- buf->resid -= bcnt;
buf->sector += bcnt >> 9;
- if (buf->resid == 0) {
- d->inprocess = NULL;
- } else if (buf->bv_resid == 0) {
- buf->bv = ++bv;
- buf->bv_resid = bv->bv_len;
- WARN_ON(buf->bv_resid == 0);
- buf->bv_off = bv->bv_offset;
- }
skb->dev = t->ifp->nd;
skb = skb_clone(skb, GFP_ATOMIC);
@@ -364,14 +392,12 @@ resend(struct aoedev *d, struct aoetgt *t, struct frame *f)
put_lba(ah, f->lba);
n = f->bcnt;
- if (n > DEFAULTBCNT)
- n = DEFAULTBCNT;
ah->scnt = n >> 9;
if (ah->aflags & AOEAFL_WRITE) {
- skb_fill_page_desc(skb, 0, virt_to_page(f->bufaddr),
- offset_in_page(f->bufaddr), n);
+ skb_fillup(skb, f->bv, f->bv_off, n);
skb->len = sizeof *h + sizeof *ah + n;
skb->data_len = n;
+ skb->truesize += n;
}
}
skb->dev = t->ifp->nd;
@@ -530,20 +556,6 @@ rexmit_timer(ulong vp)
ejectif(t, ifp);
ifp = NULL;
}
-
- if (ata_scnt(skb_mac_header(f->skb)) > DEFAULTBCNT / 512
- && ifp && ++ifp->lostjumbo > (t->nframes << 1)
- && ifp->maxbcnt != DEFAULTBCNT) {
- printk(KERN_INFO
- "aoe: e%ld.%d: "
- "too many lost jumbo on "
- "%s:%pm - "
- "falling back to %d frames.\n",
- d->aoemajor, d->aoeminor,
- ifp->nd->name, t->addr,
- DEFAULTBCNT);
- ifp->maxbcnt = 0;
- }
resend(d, t, f);
}
@@ -736,6 +748,45 @@ diskstats(struct gendisk *disk, struct bio *bio, ulong duration, sector_t sector
part_stat_unlock();
}
+static void
+bvcpy(struct bio_vec *bv, ulong off, struct sk_buff *skb, ulong cnt)
+{
+ ulong fcnt;
+ char *p;
+ int soff = 0;
+loop:
+ fcnt = bv->bv_len - (off - bv->bv_offset);
+ if (fcnt > cnt)
+ fcnt = cnt;
+ p = page_address(bv->bv_page) + off;
+ skb_copy_bits(skb, soff, p, fcnt);
+ soff += fcnt;
+ cnt -= fcnt;
+ if (cnt <= 0)
+ return;
+ bv++;
+ off = bv->bv_offset;
+ goto loop;
+}
+
+static void
+fadvance(struct frame *f, ulong cnt)
+{
+ ulong fcnt;
+
+ f->lba += cnt >> 9;
+loop:
+ fcnt = f->bv->bv_len - (f->bv_off - f->bv->bv_offset);
+ if (fcnt > cnt) {
+ f->bv_off += cnt;
+ return;
+ }
+ cnt -= fcnt;
+ f->bv++;
+ f->bv_off = f->bv->bv_offset;
+ goto loop;
+}
+
void
aoecmd_ata_rsp(struct sk_buff *skb)
{
@@ -753,6 +804,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
u16 aoemajor;
hin = (struct aoe_hdr *) skb_mac_header(skb);
+ skb_pull(skb, sizeof(*hin));
aoemajor = get_unaligned_be16(&hin->major);
d = aoedev_by_aoeaddr(aoemajor, hin->minor);
if (d == NULL) {
@@ -790,7 +842,8 @@ aoecmd_ata_rsp(struct sk_buff *skb)
calc_rttavg(d, tsince(f->tag));
- ahin = (struct aoe_atahdr *) (hin+1);
+ ahin = (struct aoe_atahdr *) skb->data;
+ skb_pull(skb, sizeof(*ahin));
hout = (struct aoe_hdr *) skb_mac_header(f->skb);
ahout = (struct aoe_atahdr *) (hout+1);
buf = f->buf;
@@ -809,7 +862,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
switch (ahout->cmdstat) {
case ATA_CMD_PIO_READ:
case ATA_CMD_PIO_READ_EXT:
- if (skb->len - sizeof *hin - sizeof *ahin < n) {
+ if (skb->len < n) {
printk(KERN_ERR
"aoe: %s. skb->len=%d need=%ld\n",
"runt data size in read", skb->len, n);
@@ -817,7 +870,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
spin_unlock_irqrestore(&d->lock, flags);
return;
}
- memcpy(f->bufaddr, ahin+1, n);
+ bvcpy(f->bv, f->bv_off, skb, n);
case ATA_CMD_PIO_WRITE:
case ATA_CMD_PIO_WRITE_EXT:
ifp = getif(t, skb->dev);
@@ -827,21 +880,22 @@ aoecmd_ata_rsp(struct sk_buff *skb)
ifp->lostjumbo = 0;
}
if (f->bcnt -= n) {
- f->lba += n >> 9;
- f->bufaddr += n;
+ fadvance(f, n);
resend(d, t, f);
goto xmit;
}
break;
case ATA_CMD_ID_ATA:
- if (skb->len - sizeof *hin - sizeof *ahin < 512) {
+ if (skb->len < 512) {
printk(KERN_INFO
"aoe: runt data size in ataid. skb->len=%d\n",
skb->len);
spin_unlock_irqrestore(&d->lock, flags);
return;
}
- ataid_complete(d, t, (char *) (ahin+1));
+ if (skb_linearize(skb))
+ break;
+ ataid_complete(d, t, skb->data);
break;
default:
printk(KERN_INFO
diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index 6b5110a..b2d1fd3 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -182,6 +182,7 @@ skbfree(struct sk_buff *skb)
"cannot free skb -- memory leaked.");
return;
}
+ skb->truesize -= skb->data_len;
skb_shinfo(skb)->nr_frags = skb->data_len = 0;
skb_trim(skb, 0);
dev_kfree_skb(skb);
diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
index 4d3bc0d..0787807 100644
--- a/drivers/block/aoe/aoenet.c
+++ b/drivers/block/aoe/aoenet.c
@@ -102,7 +102,9 @@ static int
aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt, struct net_device *orig_dev)
{
struct aoe_hdr *h;
+ struct aoe_atahdr *ah;
u32 n;
+ int sn;
if (dev_net(ifp) != &init_net)
goto exit;
@@ -110,13 +112,16 @@ aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt,
skb = skb_share_check(skb, GFP_ATOMIC);
if (skb == NULL)
return 0;
- if (skb_linearize(skb))
- goto exit;
if (!is_aoe_netif(ifp))
goto exit;
skb_push(skb, ETH_HLEN); /* (1) */
-
- h = (struct aoe_hdr *) skb_mac_header(skb);
+ sn = sizeof(*h) + sizeof(*ah);
+ if (skb->len >= sn) {
+ sn -= skb_headlen(skb);
+ if (sn > 0 && !__pskb_pull_tail(skb, sn))
+ goto exit;
+ }
+ h = (struct aoe_hdr *) skb->data;
n = get_unaligned_be32(&h->tag);
if ((h->verfl & AOEFL_RSP) == 0 || (n & 1<<31))
goto exit;
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-23 20:04 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev, josh,
eric.dumazet, mathieu.desnoyers, axboe, agk, dm-devel, neilb,
ccaulfie, teigland, Trond.Myklebust, bfields, fweisbec, jesse,
venkat.x.venkatsubra, ejt, snitzer, edumazet, linux-nfs, dev,
rds-devel, lw
In-Reply-To: <50357840.5020201@gmail.com>
Hello, Sasha.
On Thu, Aug 23, 2012 at 02:24:32AM +0200, Sasha Levin wrote:
> > I think the almost trivial nature of hlist hashtables makes this a bit
> > tricky and I'm not very sure but having this combinatory explosion is
> > a bit dazzling when the same functionality can be achieved by simply
> > combining operations which are already defined and named considering
> > hashtable. I'm not feeling too strong about this tho. What do others
> > think?
>
> I'm thinking that this hashtable API will have 2 purposes: First, it would
> prevent the excessive duplication of hashtable implementations all around the code.
>
> Second, it will allow more easily interchangeable hashtable implementations to
> find their way into the kernel. There are several maintainers who would be happy
> to see dynamically sized RCU hashtable, and I'm guessing that several more
> variants could be added based on needs in specific modules.
>
> The second reason is why several things you've mentioned look the way they are:
>
> - No DEFINE_HASHTABLE(): I wanted to force the use of hash_init() since
> initialization for other hashtables may be more complicated than the static
> initialization for this implementation, which means that any place that used
> DEFINE_HASHTABLE() and didn't do hash_init() will be buggy.
I think this is problematic. It looks exactly like other existing
DEFINE macros yet what its semantics is different. I don't think
that's a good idea.
> I'm actually tempted in hiding hlist completely from hashtable users, probably
> by simply defining a hash_head/hash_node on top of the hlist_ counterparts.
I think that it would be best to keep this one simple & obvious, which
already has enough in-kernel users to justify its existence. There
are significant benefits in being trivially understandable and
expectable. If we want more advanced ones - say resizing, hybrid or
what not, let's make that a separate one. No need to complicate the
common straight-forward case for that.
So, I think it would be best to keep this one as straight-forward and
trivial as possible. Helper macros to help its users are fine but
let's please not go for full encapsulation.
Thanks.
--
tejun
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 6/8] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Nicholas A. Bellinger @ 2012-08-23 19:58 UTC (permalink / raw)
To: Naresh Kumar Inna; +Cc: JBottomley, linux-scsi, dm, netdev, chethan
In-Reply-To: <1345760873-12101-7-git-send-email-naresh@chelsio.com>
On Fri, 2012-08-24 at 03:57 +0530, Naresh Kumar Inna wrote:
> This patch contains the first set of the header files for csiostor driver.
>
> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
> ---
> drivers/scsi/csiostor/csio_defs.h | 143 ++++++
> drivers/scsi/csiostor/csio_fcoe_proto.h | 843 +++++++++++++++++++++++++++++++
> drivers/scsi/csiostor/csio_hw.h | 668 ++++++++++++++++++++++++
> drivers/scsi/csiostor/csio_init.h | 158 ++++++
> 4 files changed, 1812 insertions(+), 0 deletions(-)
> create mode 100644 drivers/scsi/csiostor/csio_defs.h
> create mode 100644 drivers/scsi/csiostor/csio_fcoe_proto.h
> create mode 100644 drivers/scsi/csiostor/csio_hw.h
> create mode 100644 drivers/scsi/csiostor/csio_init.h
>
Hi Naresh,
Just commenting on csio_defs.h bits here... As Robert mentioned, you'll
need to convert the driver to use (or add to) upstream protocol
definitions and drop the csio_fcoe_proto.h bits..
> diff --git a/drivers/scsi/csiostor/csio_defs.h b/drivers/scsi/csiostor/csio_defs.h
> new file mode 100644
> index 0000000..4f1c713
> --- /dev/null
> +++ b/drivers/scsi/csiostor/csio_defs.h
> @@ -0,0 +1,143 @@
> +/*
> + * This file is part of the Chelsio FCoE driver for Linux.
> + *
> + * Copyright (c) 2008-2012 Chelsio Communications, Inc. All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses. You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * - Redistributions of source code must retain the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer.
> + *
> + * - Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer in the documentation and/or other materials
> + * provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef __CSIO_DEFS_H__
> +#define __CSIO_DEFS_H__
> +
> +#include <linux/kernel.h>
> +#include <linux/timer.h>
> +#include <linux/list.h>
> +#include <linux/bug.h>
> +#include <linux/pci.h>
> +#include <linux/jiffies.h>
> +
> +/* Function returns */
> +enum csio_retval {
> + CSIO_SUCCESS = 0,
> + CSIO_INVAL = 1,
> + CSIO_BUSY = 2,
> + CSIO_NOSUPP = 3,
> + CSIO_TIMEOUT = 4,
> + CSIO_NOMEM = 5,
> + CSIO_NOPERM = 6,
> + CSIO_RETRY = 7,
> + CSIO_EPROTO = 8,
> + CSIO_EIO = 9,
> + CSIO_CANCELLED = 10,
> +};
> +
Please don't assign macros for errno's, and give them positive values.
> +#define csio_retval_t enum csio_retval
Please get rid of this csio_retval_t nonsense.
> +
> +enum {
> + CSIO_FALSE = 0,
> + CSIO_TRUE = 1,
> +};
> +
Same here, please use normal Boolean macros
> +#define CSIO_ROUNDUP(__v, __r) (((__v) + (__r) - 1) / (__r))
> +#define CSIO_INVALID_IDX 0xFFFFFFFF
> +#define csio_inc_stats(elem, val) ((elem)->stats.val++)
> +#define csio_dec_stats(elem, val) ((elem)->stats.val--)
No reason for either of this stats inc+dec macros. Please drop them.
> +#define csio_valid_wwn(__n) ((*__n >> 4) == 0x5 ? CSIO_TRUE : \
> + CSIO_FALSE)
> +#define CSIO_WORD_TO_BYTE 4
> +
> +static inline int
> +csio_list_deleted(struct list_head *list)
> +{
> + return ((list->next == list) && (list->prev == list));
> +}
> +
> +#define csio_list_next(elem) (((struct list_head *)(elem))->next)
> +#define csio_list_prev(elem) (((struct list_head *)(elem))->prev)
> +
> +#define csio_deq_from_head(head, elem) \
> +do { \
> + if (!list_empty(head)) { \
> + *((struct list_head **)(elem)) = csio_list_next((head)); \
> + csio_list_next((head)) = \
> + csio_list_next(csio_list_next((head))); \
> + csio_list_prev(csio_list_next((head))) = (head); \
> + INIT_LIST_HEAD(*((struct list_head **)(elem))); \
> + } else \
> + *((struct list_head **)(elem)) = (struct list_head *)NULL;\
> +} while (0)
> +
This code is confusing as hell.. Why can't you just use normal list.h
macros for this..?
> +#define csio_deq_from_tail(head, elem) \
> +do { \
> + if (!list_empty(head)) { \
> + *((struct list_head **)(elem)) = csio_list_prev((head)); \
> + csio_list_prev((head)) = \
> + csio_list_prev(csio_list_prev((head))); \
> + csio_list_next(csio_list_prev((head))) = (head); \
> + INIT_LIST_HEAD(*((struct list_head **)(elem))); \
> + } else \
> + *((struct list_head **)(elem)) = (struct list_head *)NULL;\
> +} while (0)
> +
Same here.. Please don't use macros like this.
> +/* State machine */
> +typedef void (*csio_sm_state_t)(void *, uint32_t);
> +
> +struct csio_sm {
> + struct list_head sm_list;
> + csio_sm_state_t sm_state;
> +};
> +
> +#define csio_init_state(__smp, __state) \
> + (((struct csio_sm *)(__smp))->sm_state = (csio_sm_state_t)(__state))
> +
> +#define csio_set_state(__smp, __state) \
> + (((struct csio_sm *)(__smp))->sm_state = (csio_sm_state_t)(__state))
> +
> +
> +#define csio_post_event(__smp, __evt) \
> + (((struct csio_sm *)(__smp))->sm_state((__smp), (uint32_t)(__evt)))
> +
> +#define csio_get_state(__smp) (((struct csio_sm *)(__smp))->sm_state)
> +
> +#define csio_match_state(__smp, __state) \
> + (csio_get_state((__smp)) == (csio_sm_state_t)(__state))
> +
Why does any of the sm_state stuff need to be in macros..? Please
inline all of this code.
> +#define CSIO_ASSERT(cond) \
> +do { \
> + if (unlikely(!((cond)))) \
> + BUG(); \
> +} while (0)
> +
> +#ifdef __CSIO_DEBUG__
> +#define CSIO_DB_ASSERT(__c) CSIO_ASSERT((__c))
> +#else
> +#define CSIO_DB_ASSERT(__c)
> +#endif
> +
> +#endif /* ifndef __CSIO_DEFS_H__ */
^ permalink raw reply
* Re: [RFC PATCH bridge 0/5] Add basic VLAN support to bridges
From: Vlad Yasevich @ 2012-08-23 19:53 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20120823124141.402b7b34@nehalam.linuxnetplumber.net>
On 08/23/2012 03:41 PM, Stephen Hemminger wrote:
> On Thu, 23 Aug 2012 15:29:50 -0400
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> This series of patches provides an ability to add VLAN IDs to the bridge
>> ports. This is similar to what can be found in most switches. The bridge
>> port may have any number of VLANs added to it including vlan 0 for untagged
>> traffic. When vlans are added to the port, only traffic tagged with particular
>> vlan will forwarded over this port. Additionally, vlan ids are added to FDB
>> entries and become part of the lookup. This way we correctly identify the FDB
>> entry.
>>
>> There are still pieces missing. I don't yet support adding a static fdb entry
>> with a particular vlan. There is no netlink support for carrying a vlan id.
>>
>> I'd like to hear thoughts of whether this is usufull and something we should
>> persue.
>>
>> The default behavior ofthe bridge is unchanged if no vlans have been
>> configured.
>
> Initial reaction is that this is a useful. You can already do the same thing
> with ebtables, and ebtables allows more flexibility. But ebtables does slow
> things down, and is harder to configure.
Slowness of ebtables is exactly why I thought of doing this. This code
works pretty well when you have guests running on different vlans. It
makes sure that there is no traffic leakage.
I'll write up the netlink pieces and repost.
Thanks
-vlad
^ permalink raw reply
* Re: [PATCH 5/8] csiostor: Chelsio FCoE offload driver submission (sources part 5).
From: Nicholas A. Bellinger @ 2012-08-23 19:48 UTC (permalink / raw)
To: Naresh Kumar Inna; +Cc: JBottomley, linux-scsi, dm, netdev, chethan
In-Reply-To: <1345760873-12101-6-git-send-email-naresh@chelsio.com>
On Fri, 2012-08-24 at 03:57 +0530, Naresh Kumar Inna wrote:
> This patch contains code to implement the interrupt handling and the fast
> path I/O functionality. The interrupt handling includes allocation of
> MSIX vectors, registering and implemeting the interrupt service routines.
> The fast path I/O functionality includes posting the I/O request to firmware
> via Work Requests, tracking/completing them, and handling task management
> requests. SCSI midlayer host template implementation is also covered by
> this patch.
>
> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
> ---
Hi Naresh,
My review comments are inline below..
> drivers/scsi/csiostor/csio_isr.c | 631 ++++++++++
> drivers/scsi/csiostor/csio_scsi.c | 2498 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 3129 insertions(+), 0 deletions(-)
> create mode 100644 drivers/scsi/csiostor/csio_isr.c
> create mode 100644 drivers/scsi/csiostor/csio_scsi.c
>
> diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
> new file mode 100644
> index 0000000..96633e9
> --- /dev/null
> +++ b/drivers/scsi/csiostor/csio_isr.c
<SNIP>
> +#define csio_extra_msix_desc(_desc, _len, _str, _arg1, _arg2, _arg3) \
> +do { \
> + memset((_desc), 0, (_len) + 1); \
> + snprintf((_desc), (_len), (_str), (_arg1), (_arg2), (_arg3)); \
> +} while (0)
> +
This type of macro usage is not necessary for just two users below.
Please inline code like this.
> +static void
> +csio_add_msix_desc(struct csio_hw *hw)
> +{
> + int i;
> + struct csio_msix_entries *entryp = &hw->msix_entries[0];
> + int k = CSIO_EXTRA_VECS;
> + int len = sizeof(entryp->desc) - 1;
> + int cnt = hw->num_sqsets + k;
> +
> + /* Non-data vector */
> + csio_extra_msix_desc(entryp->desc, len, "csio-%02x:%02x:%x-nondata",
> + CSIO_PCI_BUS(hw), CSIO_PCI_DEV(hw),
> + CSIO_PCI_FUNC(hw));
> + entryp++;
> + csio_extra_msix_desc(entryp->desc, len, "csio-%02x:%02x:%x-fwevt",
> + CSIO_PCI_BUS(hw), CSIO_PCI_DEV(hw),
> + CSIO_PCI_FUNC(hw));
> + entryp++;
> +
> + /* Name SCSI vecs */
> + for (i = k; i < cnt; i++, entryp++) {
> + memset(entryp->desc, 0, len + 1);
> + snprintf(entryp->desc, len, "csio-%02x:%02x:%x-scsi%d",
> + CSIO_PCI_BUS(hw), CSIO_PCI_DEV(hw),
> + CSIO_PCI_FUNC(hw), i - CSIO_EXTRA_VECS);
> + }
> +}
> +
> diff --git a/drivers/scsi/csiostor/csio_scsi.c b/drivers/scsi/csiostor/csio_scsi.c
> new file mode 100644
> index 0000000..0f87b00
> --- /dev/null
> +++ b/drivers/scsi/csiostor/csio_scsi.c
> +
> +/*
> + * csio_scsi_match_io - Match an ioreq with the given SCSI level data.
> + * @ioreq: The I/O request
> + * @sld: Level information
> + *
> + * Should be called with lock held.
> + *
> + */
> +static bool
> +csio_scsi_match_io(struct csio_ioreq *ioreq, struct csio_scsi_level_data *sld)
> +{
> + struct scsi_cmnd *scmnd = csio_scsi_cmnd(ioreq);
> +
> + switch (sld->level) {
> + case CSIO_LEV_LUN:
> + if (scmnd == NULL)
> + return CSIO_FALSE;
> +
> + return ((ioreq->lnode == sld->lnode) &&
> + (ioreq->rnode == sld->rnode) &&
> + ((uint64_t)scmnd->device->lun == sld->oslun));
> +
> + case CSIO_LEV_RNODE:
> + return ((ioreq->lnode == sld->lnode) &&
> + (ioreq->rnode == sld->rnode));
> + case CSIO_LEV_LNODE:
> + return (ioreq->lnode == sld->lnode);
> + case CSIO_LEV_ALL:
> + return CSIO_TRUE;
> + default:
> + return CSIO_FALSE;
> + }
> +}
> +
Why can't CSIO_[TRUE,FALSE] just use normal Boolean defines..?
> +/*
> + * csio_scsi_fcp_cmnd - Frame the SCSI FCP command paylod.
> + * @req: IO req structure.
> + * @addr: DMA location to place the payload.
> + *
> + * This routine is shared between FCP_WRITE, FCP_READ and FCP_CMD requests.
> + */
> +static inline void
> +csio_scsi_fcp_cmnd(struct csio_ioreq *req, void *addr)
> +{
> + struct csio_fcp_cmnd *fcp_cmnd = (struct csio_fcp_cmnd *)addr;
> + struct scsi_cmnd *scmnd = csio_scsi_cmnd(req);
> +
> + /* Check for Task Management */
> + if (likely(scmnd->SCp.Message == 0)) {
> + int_to_scsilun(scmnd->device->lun,
> + (struct scsi_lun *)fcp_cmnd->lun);
> + fcp_cmnd->tm_flags = 0;
> + fcp_cmnd->cmdref = 0;
> + fcp_cmnd->pri_ta = 0;
> +
> + memcpy(fcp_cmnd->cdb, scmnd->cmnd, 16);
> + csio_scsi_tag(scmnd, &fcp_cmnd->pri_ta,
> + FCP_PTA_HEADQ, FCP_PTA_ORDERED, FCP_PTA_SIMPLE);
> + fcp_cmnd->dl = cpu_to_be32(scsi_bufflen(scmnd));
> +
> + if (req->nsge)
> + if (req->datadir == CSIO_IOREQF_DMA_WRITE)
The same goes for CSIO_IOREQF_DMA_*...
Why can't this just be DMA_* defs from include/linux/dma-direction.h..?
> + fcp_cmnd->flags = FCP_CFL_WRDATA;
> + else
> + fcp_cmnd->flags = FCP_CFL_RDDATA;
> + else
> + fcp_cmnd->flags = 0;
> + } else {
> + memset(fcp_cmnd, 0, sizeof(*fcp_cmnd));
> + int_to_scsilun(scmnd->device->lun,
> + (struct scsi_lun *)fcp_cmnd->lun);
> + fcp_cmnd->tm_flags = (uint8_t)scmnd->SCp.Message;
> + }
> +}
> +
> +
> +#define CSIO_SCSI_CMD_WR_SZ(_imm) \
> + (sizeof(struct fw_scsi_cmd_wr) + /* WR size */ \
> + ALIGN((_imm), 16)) /* Immed data */
> +
> +#define CSIO_SCSI_CMD_WR_SZ_16(_imm) \
> + (ALIGN(CSIO_SCSI_CMD_WR_SZ((_imm)), 16))
> +
> +/*
> + * csio_scsi_cmd - Create a SCSI CMD WR.
> + * @req: IO req structure.
> + *
> + * Gets a WR slot in the ingress queue and initializes it with SCSI CMD WR.
> + *
> + */
> +static inline void
> +csio_scsi_cmd(struct csio_ioreq *req)
> +{
> + struct csio_wr_pair wrp;
> + struct csio_hw *hw = req->lnode->hwp;
> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
> + uint32_t size = CSIO_SCSI_CMD_WR_SZ_16(scsim->proto_cmd_len);
> +
> + req->drv_status = csio_wr_get(hw, req->eq_idx, size, &wrp);
> + if (unlikely(req->drv_status != CSIO_SUCCESS))
> + return;
> +
> + if (wrp.size1 >= size) {
> + /* Initialize WR in one shot */
> + csio_scsi_init_cmd_wr(req, wrp.addr1, size);
> + } else {
> + uint8_t tmpwr[512];
Mmmm, putting this large of a buffer on the local stack is probably not
a good idea.
This should become an allocation.. If it's a hot path then you'll
probably want to set this up before-hand.
> + /*
> + * Make a temporary copy of the WR and write back
> + * the copy into the WR pair.
> + */
> + csio_scsi_init_cmd_wr(req, (void *)tmpwr, size);
> + memcpy(wrp.addr1, tmpwr, wrp.size1);
> + memcpy(wrp.addr2, tmpwr + wrp.size1, size - wrp.size1);
> + }
> +}
> +
> +/*
> + * The following is fast path code. Therefore it is inlined with multi-line
> + * macros using name substitution, thus avoiding if-else switches for
> + * operation (read/write), as well as serving the purpose of code re-use.
> + */
> +/*
> + * csio_scsi_init_ulptx_dsgl - Fill in a ULP_TX_SC_DSGL
> + * @hw: HW module
> + * @req: IO request
> + * @sgl: ULP TX SGL pointer.
> + *
> + */
> +#define csio_scsi_init_ultptx_dsgl(hw, req, sgl) \
> +do { \
> + struct ulptx_sge_pair *_sge_pair = NULL; \
> + struct scatterlist *_sgel; \
> + uint32_t _i = 0; \
> + uint32_t _xfer_len; \
> + struct list_head *_tmp; \
> + struct csio_dma_buf *_dma_buf; \
> + struct scsi_cmnd *scmnd = csio_scsi_cmnd((req)); \
> + \
> + (sgl)->cmd_nsge = htonl(ULPTX_CMD(ULP_TX_SC_DSGL) | ULPTX_MORE | \
> + ULPTX_NSGE((req)->nsge)); \
> + /* Now add the data SGLs */ \
> + if (likely(!(req)->dcopy)) { \
> + scsi_for_each_sg(scmnd, _sgel, (req)->nsge, _i) { \
> + if (_i == 0) { \
> + (sgl)->addr0 = cpu_to_be64( \
> + sg_dma_address(_sgel)); \
> + (sgl)->len0 = cpu_to_be32( \
> + sg_dma_len(_sgel)); \
> + _sge_pair = \
> + (struct ulptx_sge_pair *)((sgl) + 1); \
> + continue; \
> + } \
> + if ((_i - 1) & 0x1) { \
> + _sge_pair->addr[1] = cpu_to_be64( \
> + sg_dma_address(_sgel)); \
> + _sge_pair->len[1] = cpu_to_be32( \
> + sg_dma_len(_sgel)); \
> + _sge_pair++; \
> + } else { \
> + _sge_pair->addr[0] = cpu_to_be64( \
> + sg_dma_address(_sgel)); \
> + _sge_pair->len[0] = cpu_to_be32( \
> + sg_dma_len(_sgel)); \
> + } \
> + } \
> + } else { \
> + /* Program sg elements with driver's DDP buffer */ \
> + _xfer_len = scsi_bufflen(scmnd); \
> + list_for_each(_tmp, &(req)->gen_list) { \
> + _dma_buf = (struct csio_dma_buf *)_tmp; \
> + if (_i == 0) { \
> + (sgl)->addr0 = cpu_to_be64(_dma_buf->paddr); \
> + (sgl)->len0 = cpu_to_be32( \
> + min(_xfer_len, _dma_buf->len)); \
> + _sge_pair = \
> + (struct ulptx_sge_pair *)((sgl) + 1); \
> + } \
> + else if ((_i - 1) & 0x1) { \
> + _sge_pair->addr[1] = cpu_to_be64( \
> + _dma_buf->paddr); \
> + _sge_pair->len[1] = cpu_to_be32( \
> + min(_xfer_len, _dma_buf->len)); \
> + _sge_pair++; \
> + } else { \
> + _sge_pair->addr[0] = cpu_to_be64( \
> + _dma_buf->paddr); \
> + _sge_pair->len[0] = cpu_to_be32( \
> + min(_xfer_len, _dma_buf->len)); \
> + } \
> + _xfer_len -= min(_xfer_len, _dma_buf->len); \
> + _i++; \
> + } \
> + } \
> +} while (0)
> +
I don't see any reason why this can't just be a static function..? Why
is the macro usage necessary here..?
> +/*
> + * csio_scsi_init_data_wr - Initialize the READ/WRITE SCSI WR.
> + * @req: IO req structure.
> + * @oper: read/write
> + * @wrp: DMA location to place the payload.
> + * @size: Size of WR (including FW WR + immed data + rsp SG entry + data SGL
> + * @wrop: _READ_/_WRITE_
> + *
> + * Wrapper for populating fw_scsi_read_wr/fw_scsi_write_wr.
> + */
> +#define csio_scsi_init_data_wr(req, oper, wrp, size, wrop) \
> +do { \
> + struct csio_hw *_hw = (req)->lnode->hwp; \
> + struct csio_rnode *_rn = (req)->rnode; \
> + struct fw_scsi_##oper##_wr *__wr = (struct fw_scsi_##oper##_wr *)(wrp);\
> + struct ulptx_sgl *_sgl; \
> + struct csio_dma_buf *_dma_buf; \
> + uint8_t _imm = csio_hw_to_scsim(_hw)->proto_cmd_len; \
> + struct scsi_cmnd *scmnd = csio_scsi_cmnd((req)); \
> + \
> + __wr->op_immdlen = cpu_to_be32(FW_WR_OP(FW_SCSI##wrop##WR) | \
> + FW_SCSI##wrop##WR_IMMDLEN(_imm)); \
> + __wr->flowid_len16 = cpu_to_be32(FW_WR_FLOWID(_rn->flowid) | \
> + FW_WR_LEN16( \
> + CSIO_ROUNDUP((size), 16))); \
> + __wr->cookie = (uintptr_t) (req); \
> + __wr->iqid = (uint16_t)cpu_to_be16(csio_q_physiqid(_hw, \
> + (req)->iq_idx));\
> + __wr->tmo_val = (uint8_t)((req)->tmo); \
> + __wr->use_xfer_cnt = 1; \
> + __wr->xfer_cnt = cpu_to_be32(scsi_bufflen(scmnd)); \
> + __wr->ini_xfer_cnt = cpu_to_be32(scsi_bufflen(scmnd)); \
> + /* Get RSP DMA buffer */ \
> + _dma_buf = &(req)->dma_buf; \
> + \
> + /* Prepare RSP SGL */ \
> + __wr->rsp_dmalen = cpu_to_be32(_dma_buf->len); \
> + __wr->rsp_dmaaddr = cpu_to_be64(_dma_buf->paddr); \
> + \
> + __wr->r4 = 0; \
> + \
> + __wr->u.fcoe.ctl_pri = 0; \
> + __wr->u.fcoe.cp_en_class = 0; \
> + __wr->u.fcoe.r3_lo[0] = 0; \
> + __wr->u.fcoe.r3_lo[1] = 0; \
> + csio_scsi_fcp_cmnd((req), (void *)((uintptr_t)(wrp) + \
> + sizeof(struct fw_scsi_##oper##_wr))); \
> + \
> + /* Move WR pointer past command and immediate data */ \
> + _sgl = (struct ulptx_sgl *) ((uintptr_t)(wrp) + \
> + sizeof(struct fw_scsi_##oper##_wr) + \
> + ALIGN(_imm, 16)); \
> + \
> + /* Fill in the DSGL */ \
> + csio_scsi_init_ultptx_dsgl(_hw, (req), _sgl); \
> + \
> +} while (0)
> +
This one has four uses of CPP keys. Just turn those into macros, and
leave the rest of the code in a static function.
> +/* Calculate WR size needed for fw_scsi_read_wr/fw_scsi_write_wr */
> +#define csio_scsi_data_wrsz(req, oper, sz, imm) \
> +do { \
> + (sz) = sizeof(struct fw_scsi_##oper##_wr) + /* WR size */ \
> + ALIGN((imm), 16) + /* Immed data */ \
> + sizeof(struct ulptx_sgl); /* ulptx_sgl */ \
> + \
> + if (unlikely((req)->nsge > 1)) \
> + (sz) += (sizeof(struct ulptx_sge_pair) * \
> + (ALIGN(((req)->nsge - 1), 2) / 2)); \
> + /* Data SGE */ \
> +} while (0)
> +
> +/*
> + * csio_scsi_data - Create a SCSI WRITE/READ WR.
> + * @req: IO req structure.
> + * @oper: read/write
> + * @wrop: _READ_/_WRITE_ (string subsitutions to use with the FW bit field
> + * macros).
> + *
> + * Gets a WR slot in the ingress queue and initializes it with
> + * SCSI CMD READ/WRITE WR.
> + *
> + */
> +#define csio_scsi_data(req, oper, wrop) \
> +do { \
> + struct csio_wr_pair _wrp; \
> + uint32_t _size; \
> + struct csio_hw *_hw = (req)->lnode->hwp; \
> + struct csio_scsim *_scsim = csio_hw_to_scsim(_hw); \
> + \
> + csio_scsi_data_wrsz((req), oper, _size, _scsim->proto_cmd_len); \
> + _size = ALIGN(_size, 16); \
> + \
> + (req)->drv_status = csio_wr_get(_hw, (req)->eq_idx, _size, &_wrp); \
> + if (likely((req)->drv_status == CSIO_SUCCESS)) { \
> + if (likely(_wrp.size1 >= _size)) { \
> + /* Initialize WR in one shot */ \
> + csio_scsi_init_data_wr((req), oper, _wrp.addr1, \
> + _size, wrop); \
> + } else { \
> + uint8_t tmpwr[512]; \
> + /* \
> + * Make a temporary copy of the WR and write back \
> + * the copy into the WR pair. \
> + */ \
> + csio_scsi_init_data_wr((req), oper, (void *)tmpwr, \
> + _size, wrop); \
> + memcpy(_wrp.addr1, tmpwr, _wrp.size1); \
> + memcpy(_wrp.addr2, tmpwr + _wrp.size1, \
> + _size - _wrp.size1); \
> + } \
> + } \
> +} while (0)
> +
Ditto on this one, along with the tmpwr[512] stack usage..
> +static inline void
> +csio_scsi_abrt_cls(struct csio_ioreq *req, bool abort)
> +{
> + struct csio_wr_pair wrp;
> + struct csio_hw *hw = req->lnode->hwp;
> + uint32_t size = ALIGN(sizeof(struct fw_scsi_abrt_cls_wr), 16);
> +
> + req->drv_status = csio_wr_get(hw, req->eq_idx, size, &wrp);
> + if (req->drv_status != CSIO_SUCCESS)
> + return;
> +
> + if (wrp.size1 >= size) {
> + /* Initialize WR in one shot */
> + csio_scsi_init_abrt_cls_wr(req, wrp.addr1, size, abort);
> + } else {
> + uint8_t tmpwr[512];
Ditto here on local scope stack usage..
> + /*
> + * Make a temporary copy of the WR and write back
> + * the copy into the WR pair.
> + */
> + csio_scsi_init_abrt_cls_wr(req, (void *)tmpwr, size, abort);
> + memcpy(wrp.addr1, tmpwr, wrp.size1);
> + memcpy(wrp.addr2, tmpwr + wrp.size1, size - wrp.size1);
> + }
> +}
> +
> +/*****************************************************************************/
> +/* START: SCSI SM */
> +/*****************************************************************************/
> +static void
> +csio_scsis_uninit(struct csio_ioreq *req, enum csio_scsi_ev evt)
> +{
> + struct csio_hw *hw = req->lnode->hwp;
> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
> +
> + switch (evt) {
> +
> + case CSIO_SCSIE_START_IO:
Extra space between start of first switch case
> +
> + /* There is data */
Point-less comment
> + if (req->nsge) {
> + if (req->datadir == CSIO_IOREQF_DMA_WRITE) {
> + req->dcopy = 0;
> + csio_scsi_data(req, write, _WRITE_);
> + } else
> + csio_setup_ddp(scsim, req);
> + } else {
> + csio_scsi_cmd(req);
> + }
> +
> + if (likely(req->drv_status == CSIO_SUCCESS)) {
> + /* change state and enqueue on active_q */
> + csio_set_state(&req->sm, csio_scsis_io_active);
> + list_add_tail(&req->sm.sm_list, &scsim->active_q);
> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
> + csio_inc_stats(scsim, n_active);
> +
> + return;
> + }
> + break;
> +
> + case CSIO_SCSIE_START_TM:
> + csio_scsi_cmd(req);
> + if (req->drv_status == CSIO_SUCCESS) {
> + /*
> + * NOTE: We collect the affected I/Os prior to issuing
> + * LUN reset, and not after it. This is to prevent
> + * aborting I/Os that get issued after the LUN reset,
> + * but prior to LUN reset completion (in the event that
> + * the host stack has not blocked I/Os to a LUN that is
> + * being reset.
> + */
> + csio_set_state(&req->sm, csio_scsis_tm_active);
> + list_add_tail(&req->sm.sm_list, &scsim->active_q);
> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
> + csio_inc_stats(scsim, n_tm_active);
> + }
> + return;
> +
> + case CSIO_SCSIE_ABORT:
> + case CSIO_SCSIE_CLOSE:
> + /*
> + * NOTE:
> + * We could get here due to :
> + * - a window in the cleanup path of the SCSI module
> + * (csio_scsi_abort_io()). Please see NOTE in this function.
> + * - a window in the time we tried to issue an abort/close
> + * of a request to FW, and the FW completed the request
> + * itself.
> + * Print a message for now, and return INVAL either way.
> + */
> + req->drv_status = CSIO_INVAL;
> + csio_warn(hw, "Trying to abort/close completed IO:%p!\n", req);
> + break;
> +
> + default:
> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
> + CSIO_DB_ASSERT(0);
> + }
> +}
> +
> +static void
> +csio_scsis_io_active(struct csio_ioreq *req, enum csio_scsi_ev evt)
> +{
> + struct csio_hw *hw = req->lnode->hwp;
> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
> + struct csio_rnode *rn;
> +
> + switch (evt) {
> +
> + case CSIO_SCSIE_COMPLETED:
Ditto
> + csio_dec_stats(scm, n_active);
> + list_del_init(&req->sm.sm_list);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + /*
> + * In MSIX mode, with multiple queues, the SCSI compeltions
> + * could reach us sooner than the FW events sent to indicate
> + * I-T nexus loss (link down, remote device logo etc). We
> + * dont want to be returning such I/Os to the upper layer
> + * immediately, since we wouldnt have reported the I-T nexus
> + * loss itself. This forces us to serialize such completions
> + * with the reporting of the I-T nexus loss. Therefore, we
> + * internally queue up such up such completions in the rnode.
> + * The reporting of I-T nexus loss to the upper layer is then
> + * followed by the returning of I/Os in this internal queue.
> + * Having another state alongwith another queue helps us take
> + * actions for events such as ABORT received while we are
> + * in this rnode queue.
> + */
> + if (unlikely(req->wr_status != FW_SUCCESS)) {
> + rn = req->rnode;
> + /*
> + * FW says remote device is lost, but rnode
> + * doesnt reflect it.
> + */
> + if (csio_scsi_itnexus_loss_error(req->wr_status) &&
> + csio_is_rnode_ready(rn)) {
> + csio_set_state(&req->sm,
> + csio_scsis_shost_cmpl_await);
> + list_add_tail(&req->sm.sm_list,
> + &rn->host_cmpl_q);
> + }
> + }
> +
> + break;
> +
> + case CSIO_SCSIE_ABORT:
> + csio_scsi_abrt_cls(req, SCSI_ABORT);
> + if (req->drv_status == CSIO_SUCCESS) {
> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
> + csio_set_state(&req->sm, csio_scsis_aborting);
> + }
> + break;
> +
> + case CSIO_SCSIE_CLOSE:
> + csio_scsi_abrt_cls(req, SCSI_CLOSE);
> + if (req->drv_status == CSIO_SUCCESS) {
> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
> + csio_set_state(&req->sm, csio_scsis_closing);
> + }
> + break;
> +
> + case CSIO_SCSIE_DRVCLEANUP:
> + req->wr_status = FW_HOSTERROR;
> + csio_dec_stats(scm, n_active);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + break;
> +
> + default:
> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
> + CSIO_DB_ASSERT(0);
> + }
> +}
> +
> +static void
> +csio_scsis_tm_active(struct csio_ioreq *req, enum csio_scsi_ev evt)
> +{
> + struct csio_hw *hw = req->lnode->hwp;
> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
> +
> + switch (evt) {
> +
> + case CSIO_SCSIE_COMPLETED:
Ditto
> + csio_dec_stats(scm, n_tm_active);
> + list_del_init(&req->sm.sm_list);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> +
> + break;
> +
> + case CSIO_SCSIE_ABORT:
> + csio_scsi_abrt_cls(req, SCSI_ABORT);
> + if (req->drv_status == CSIO_SUCCESS) {
> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
> + csio_set_state(&req->sm, csio_scsis_aborting);
> + }
> + break;
> +
> +
> + case CSIO_SCSIE_CLOSE:
> + csio_scsi_abrt_cls(req, SCSI_CLOSE);
> + if (req->drv_status == CSIO_SUCCESS) {
> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
> + csio_set_state(&req->sm, csio_scsis_closing);
> + }
> + break;
> +
> + case CSIO_SCSIE_DRVCLEANUP:
> + req->wr_status = FW_HOSTERROR;
> + csio_dec_stats(scm, n_tm_active);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + break;
> +
> + default:
> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
> + CSIO_DB_ASSERT(0);
> + }
> +}
> +
> +static void
> +csio_scsis_aborting(struct csio_ioreq *req, enum csio_scsi_ev evt)
> +{
> + struct csio_hw *hw = req->lnode->hwp;
> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
> +
> + switch (evt) {
> +
> + case CSIO_SCSIE_COMPLETED:
Ditto
> + csio_dbg(hw,
> + "ioreq %p recvd cmpltd (wr_status:%d) "
> + "in aborting st\n", req, req->wr_status);
> + /*
> + * Use CSIO_CANCELLED to explicitly tell the ABORTED event that
> + * the original I/O was returned to driver by FW.
> + * We dont really care if the I/O was returned with success by
> + * FW (because the ABORT and completion of the I/O crossed each
> + * other), or any other return value. Once we are in aborting
> + * state, the success or failure of the I/O is unimportant to
> + * us.
> + */
> + req->drv_status = CSIO_CANCELLED;
> + break;
> +
> + case CSIO_SCSIE_ABORT:
> + csio_inc_stats(scm, n_abrt_dups);
> + break;
> +
> + case CSIO_SCSIE_ABORTED:
> +
> + csio_dbg(hw, "abort of %p return status:0x%x drv_status:%x\n",
> + req, req->wr_status, req->drv_status);
> + /*
> + * Check if original I/O WR completed before the Abort
> + * completion.
> + */
> + if (req->drv_status != CSIO_CANCELLED) {
> + csio_fatal(hw,
> + "Abort completed before original I/O,"
> + " req:%p\n", req);
> + CSIO_DB_ASSERT(0);
> + }
> +
> + /*
> + * There are the following possible scenarios:
> + * 1. The abort completed successfully, FW returned FW_SUCCESS.
> + * 2. The completion of an I/O and the receipt of
> + * abort for that I/O by the FW crossed each other.
> + * The FW returned FW_EINVAL. The original I/O would have
> + * returned with FW_SUCCESS or any other SCSI error.
> + * 3. The FW couldnt sent the abort out on the wire, as there
> + * was an I-T nexus loss (link down, remote device logged
> + * out etc). FW sent back an appropriate IT nexus loss status
> + * for the abort.
> + * 4. FW sent an abort, but abort timed out (remote device
> + * didnt respond). FW replied back with
> + * FW_SCSI_ABORT_TIMEDOUT.
> + * 5. FW couldnt genuinely abort the request for some reason,
> + * and sent us an error.
> + *
> + * The first 3 scenarios are treated as succesful abort
> + * operations by the host, while the last 2 are failed attempts
> + * to abort. Manipulate the return value of the request
> + * appropriately, so that host can convey these results
> + * back to the upper layer.
> + */
> + if ((req->wr_status == FW_SUCCESS) ||
> + (req->wr_status == FW_EINVAL) ||
> + csio_scsi_itnexus_loss_error(req->wr_status))
> + req->wr_status = FW_SCSI_ABORT_REQUESTED;
> +
> + csio_dec_stats(scm, n_active);
> + list_del_init(&req->sm.sm_list);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + break;
> +
> + case CSIO_SCSIE_DRVCLEANUP:
> + req->wr_status = FW_HOSTERROR;
> + csio_dec_stats(scm, n_active);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + break;
> +
> + case CSIO_SCSIE_CLOSE:
> + /*
> + * We can receive this event from the module
> + * cleanup paths, if the FW forgot to reply to the ABORT WR
> + * and left this ioreq in this state. For now, just ignore
> + * the event. The CLOSE event is sent to this state, as
> + * the LINK may have already gone down.
> + */
> + break;
> +
> + default:
> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
> + CSIO_DB_ASSERT(0);
> + }
> +}
> +
> +static void
> +csio_scsis_closing(struct csio_ioreq *req, enum csio_scsi_ev evt)
> +{
> + struct csio_hw *hw = req->lnode->hwp;
> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
> +
> + switch (evt) {
> +
> + case CSIO_SCSIE_COMPLETED:
Ditto
> + csio_dbg(hw,
> + "ioreq %p recvd cmpltd (wr_status:%d) "
> + "in closing st\n", req, req->wr_status);
> + /*
> + * Use CSIO_CANCELLED to explicitly tell the CLOSED event that
> + * the original I/O was returned to driver by FW.
> + * We dont really care if the I/O was returned with success by
> + * FW (because the CLOSE and completion of the I/O crossed each
> + * other), or any other return value. Once we are in aborting
> + * state, the success or failure of the I/O is unimportant to
> + * us.
> + */
> + req->drv_status = CSIO_CANCELLED;
> + break;
> +
> + case CSIO_SCSIE_CLOSED:
> + /*
> + * Check if original I/O WR completed before the Close
> + * completion.
> + */
> + if (req->drv_status != CSIO_CANCELLED) {
> + csio_fatal(hw,
> + "Close completed before original I/O,"
> + " req:%p\n", req);
> + CSIO_DB_ASSERT(0);
> + }
> +
> + /*
> + * Either close succeeded, or we issued close to FW at the
> + * same time FW compelted it to us. Either way, the I/O
> + * is closed.
> + */
> + CSIO_DB_ASSERT((req->wr_status == FW_SUCCESS) ||
> + (req->wr_status == FW_EINVAL));
> + req->wr_status = FW_SCSI_CLOSE_REQUESTED;
> +
> + csio_dec_stats(scm, n_active);
> + list_del_init(&req->sm.sm_list);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + break;
> +
> + case CSIO_SCSIE_CLOSE:
> + break;
> +
> + case CSIO_SCSIE_DRVCLEANUP:
> + req->wr_status = FW_HOSTERROR;
> + csio_dec_stats(scm, n_active);
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + break;
> +
> + default:
> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
> + CSIO_DB_ASSERT(0);
> + }
> +}
> +
> +static void
> +csio_scsis_shost_cmpl_await(struct csio_ioreq *req, enum csio_scsi_ev evt)
> +{
> + switch (evt) {
> + case CSIO_SCSIE_ABORT:
> + case CSIO_SCSIE_CLOSE:
> + /*
> + * Just succeed the abort request, and hope that
> + * the remote device unregister path will cleanup
> + * this I/O to the upper layer within a sane
> + * amount of time.
> + */
> + /*
> + * A close can come in during a LINK DOWN. The FW would have
> + * returned us the I/O back, but not the remote device lost
> + * FW event. In this interval, if the I/O times out at the upper
> + * layer, a close can come in. Take the same action as abort:
> + * return success, and hope that the remote device unregister
> + * path will cleanup this I/O. If the FW still doesnt send
> + * the msg, the close times out, and the upper layer resorts
> + * to the next level of error recovery.
> + */
> + req->drv_status = CSIO_SUCCESS;
> + break;
> + case CSIO_SCSIE_DRVCLEANUP:
> + csio_set_state(&req->sm, csio_scsis_uninit);
> + break;
> + default:
> + csio_dbg(req->lnode->hwp, "Unhandled event:%d sent to req:%p\n",
> + evt, req);
> + CSIO_DB_ASSERT(0);
> + }
> +}
> +
> +
> +/**
> + * csio_queuecommand_lck - Entry point to kickstart an I/O request.
> + * @cmnd: The I/O request from ML.
> + * @done: The ML callback routine.
> + *
> + * This routine does the following:
> + * - Checks for HW and Rnode module readiness.
> + * - Gets a free ioreq structure (which is already initialized
> + * to uninit during its allocation).
> + * - Maps SG elements.
> + * - Initializes ioreq members.
> + * - Kicks off the SCSI state machine for this IO.
> + * - Returns busy status on error.
> + */
> +static int
> +csio_queuecommand_lck(struct scsi_cmnd *cmnd, void (*done)(struct scsi_cmnd *))
> +{
> + struct csio_lnode *ln = shost_priv(cmnd->device->host);
> + struct csio_hw *hw = csio_lnode_to_hw(ln);
> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
> + struct csio_rnode *rn = (struct csio_rnode *)(cmnd->device->hostdata);
> + struct csio_ioreq *ioreq = NULL;
> + unsigned long flags;
> + int nsge = 0;
> + int rv = SCSI_MLQUEUE_HOST_BUSY, nr;
> + csio_retval_t retval;
> + int cpu;
> + struct csio_scsi_qset *sqset;
> + struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device));
> +
> + if (!blk_rq_cpu_valid(cmnd->request))
> + cpu = smp_processor_id();
> + else
> + cpu = cmnd->request->cpu;
> +
> + sqset = &hw->sqset[ln->portid][cpu];
> +
> + nr = fc_remote_port_chkready(rport);
> + if (nr) {
> + cmnd->result = nr;
> + csio_inc_stats(scsim, n_rn_nr_error);
> + goto err_done;
> + }
> +
> + if (unlikely(!csio_is_hw_ready(hw))) {
> + cmnd->result = (DID_REQUEUE << 16);
> + csio_inc_stats(scsim, n_hw_nr_error);
> + goto err_done;
> + }
> +
> + /* Get req->nsge, if there are SG elements to be mapped */
> + nsge = scsi_dma_map(cmnd);
> + if (unlikely(nsge < 0)) {
> + csio_inc_stats(scsim, n_dmamap_error);
> + goto err;
> + }
> +
> + /* Do we support so many mappings? */
> + if (unlikely(nsge > scsim->max_sge)) {
> + csio_warn(hw,
> + "More SGEs than can be supported."
> + " SGEs: %d, Max SGEs: %d\n", nsge, scsim->max_sge);
> + csio_inc_stats(scsim, n_unsupp_sge_error);
> + goto err_dma_unmap;
> + }
> +
> + /* Get a free ioreq structure - SM is already set to uninit */
> + ioreq = csio_get_scsi_ioreq_lock(hw, scsim);
> + if (!ioreq) {
> + csio_err(hw, "Out of I/O request elements. Active #:%d\n",
> + scsim->stats.n_active);
> + csio_inc_stats(scsim, n_no_req_error);
> + goto err_dma_unmap;
> + }
> +
> + ioreq->nsge = nsge;
> + ioreq->lnode = ln;
> + ioreq->rnode = rn;
> + ioreq->iq_idx = sqset->iq_idx;
> + ioreq->eq_idx = sqset->eq_idx;
> + ioreq->wr_status = 0;
> + ioreq->drv_status = CSIO_SUCCESS;
> + csio_scsi_cmnd(ioreq) = (void *)cmnd;
> + ioreq->tmo = 0;
> +
> + switch (cmnd->sc_data_direction) {
> + case DMA_BIDIRECTIONAL:
> + ioreq->datadir = CSIO_IOREQF_DMA_BIDI;
> + csio_inc_stats(ln, n_control_requests);
> + break;
> + case DMA_TO_DEVICE:
> + ioreq->datadir = CSIO_IOREQF_DMA_WRITE;
> + csio_inc_stats(ln, n_output_requests);
> + ln->stats.n_output_bytes += scsi_bufflen(cmnd);
> + break;
> + case DMA_FROM_DEVICE:
> + ioreq->datadir = CSIO_IOREQF_DMA_READ;
> + csio_inc_stats(ln, n_input_requests);
> + ln->stats.n_input_bytes += scsi_bufflen(cmnd);
> + break;
> + case DMA_NONE:
> + ioreq->datadir = CSIO_IOREQF_DMA_NONE;
> + csio_inc_stats(ln, n_control_requests);
> + break;
> + default:
> + CSIO_DB_ASSERT(0);
> + break;
> + }
> +
> + /* Set cbfn */
> + ioreq->io_cbfn = csio_scsi_cbfn;
> +
> + /* Needed during abort */
> + cmnd->host_scribble = (unsigned char *)ioreq;
> + cmnd->scsi_done = done;
> + cmnd->SCp.Message = 0;
> +
> + /* Kick off SCSI IO SM on the ioreq */
> + spin_lock_irqsave(&hw->lock, flags);
> + retval = csio_scsi_start_io(ioreq);
> + spin_unlock_irqrestore(&hw->lock, flags);
> +
> + if (retval != CSIO_SUCCESS) {
> + csio_err(hw, "ioreq: %p couldnt be started, status:%d\n",
> + ioreq, retval);
> + csio_inc_stats(scsim, n_busy_error);
> + goto err_put_req;
> + }
> +
> + return 0;
> +
> +err_put_req:
> + csio_put_scsi_ioreq_lock(hw, scsim, ioreq);
> +err_dma_unmap:
> + if (nsge > 0)
> + scsi_dma_unmap(cmnd);
> +err:
> + return rv;
> +
> +err_done:
> + done(cmnd);
> + return 0;
> +}
> +
> +static DEF_SCSI_QCMD(csio_queuecommand);
> +
This means that your running with the host_lock held.. I'm not sure if
that is really what you want to do as it really end's up killing
multi-lun small packet performance..
How about dropping DEF_SCSI_QCMD usage here, and figure out what
actually needs to be protected by the SCSI host_lock within
csio_queuecommand_lck()..?
> +static csio_retval_t
> +csio_do_abrt_cls(struct csio_hw *hw, struct csio_ioreq *ioreq, bool abort)
> +{
> + csio_retval_t rv;
> + int cpu = smp_processor_id();
> + struct csio_lnode *ln = ioreq->lnode;
> + struct csio_scsi_qset *sqset = &hw->sqset[ln->portid][cpu];
> +
> + ioreq->tmo = CSIO_SCSI_ABRT_TMO_MS;
> + /*
> + * Use current processor queue for posting the abort/close, but retain
> + * the ingress queue ID of the original I/O being aborted/closed - we
> + * need the abort/close completion to be received on the same queue
> + * as the original I/O.
> + */
> + ioreq->eq_idx = sqset->eq_idx;
> +
> + if (abort == SCSI_ABORT)
> + rv = csio_scsi_abort(ioreq);
> + else /* close */
Point-less comment.
> + rv = csio_scsi_close(ioreq);
> +
> + return rv;
> +}
> +
> +static int
> +csio_eh_abort_handler(struct scsi_cmnd *cmnd)
> +{
> + struct csio_ioreq *ioreq;
> + struct csio_lnode *ln = shost_priv(cmnd->device->host);
> + struct csio_hw *hw = csio_lnode_to_hw(ln);
> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
> + int ready = 0, ret;
> + unsigned long tmo = 0;
> + csio_retval_t rv;
> + struct csio_rnode *rn = (struct csio_rnode *)(cmnd->device->hostdata);
> +
> + ret = fc_block_scsi_eh(cmnd);
> + if (ret)
> + return ret;
> +
> + ioreq = (struct csio_ioreq *)cmnd->host_scribble;
> + if (!ioreq)
> + return SUCCESS;
> +
> + if (!rn)
> + return FAILED;
> +
> + csio_dbg(hw,
> + "Request to abort ioreq:%p cmd:%p cdb:%08llx"
> + " ssni:0x%x lun:%d iq:0x%x\n",
> + ioreq, cmnd, *((uint64_t *)cmnd->cmnd), rn->flowid,
> + cmnd->device->lun, csio_q_physiqid(hw, ioreq->iq_idx));
> +
> + if (((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) != cmnd) {
> + csio_inc_stats(scsim, n_abrt_race_comp);
> + return SUCCESS;
> + }
> +
> + ready = csio_is_lnode_ready(ln);
> + tmo = CSIO_SCSI_ABRT_TMO_MS;
> +
> + spin_lock_irq(&hw->lock);
> + rv = csio_do_abrt_cls(hw, ioreq, (ready ? SCSI_ABORT : SCSI_CLOSE));
> + spin_unlock_irq(&hw->lock);
> +
> + if (rv != CSIO_SUCCESS) {
> + if (rv == CSIO_INVAL) {
> + /* Return success, if abort/close request issued on
> + * already completed IO
> + */
> + return SUCCESS;
> + }
> + if (ready)
> + csio_inc_stats(scsim, n_abrt_busy_error);
> + else
> + csio_inc_stats(scsim, n_cls_busy_error);
> +
> + goto inval_scmnd;
> + }
> +
> + /* Wait for completion */
> + init_completion(&ioreq->cmplobj);
> + wait_for_completion_timeout(&ioreq->cmplobj, msecs_to_jiffies(tmo));
> +
> + /* FW didnt respond to abort within our timeout */
> + if (((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) == cmnd) {
> +
> + csio_err(hw, "Abort timed out -- req: %p\n", ioreq);
> + csio_inc_stats(scsim, n_abrt_timedout);
> +
> +inval_scmnd:
> + if (ioreq->nsge > 0)
> + scsi_dma_unmap(cmnd);
> +
> + spin_lock_irq(&hw->lock);
> + csio_scsi_cmnd(ioreq) = NULL;
> + spin_unlock_irq(&hw->lock);
> +
> + cmnd->result = (DID_ERROR << 16);
> + cmnd->scsi_done(cmnd);
> +
> + return FAILED;
> + }
> +
> + /* FW successfully aborted the request */
> + if (host_byte(cmnd->result) == DID_REQUEUE) {
> + csio_info(hw,
> + "Aborted SCSI command to (%d:%d) serial#:0x%lx\n",
> + cmnd->device->id, cmnd->device->lun,
> + cmnd->serial_number);
> + return SUCCESS;
> + } else {
> + csio_info(hw,
> + "Failed to abort SCSI command, (%d:%d) serial#:0x%lx\n",
> + cmnd->device->id, cmnd->device->lun,
> + cmnd->serial_number);
> + return FAILED;
> + }
> +}
> +
> +struct scsi_host_template csio_fcoe_shost_template = {
> + .module = THIS_MODULE,
> + .name = CSIO_DRV_DESC,
> + .proc_name = KBUILD_MODNAME,
> + .queuecommand = csio_queuecommand,
> + .eh_abort_handler = csio_eh_abort_handler,
> + .eh_device_reset_handler = csio_eh_lun_reset_handler,
> + .slave_alloc = csio_slave_alloc,
> + .slave_configure = csio_slave_configure,
> + .slave_destroy = csio_slave_destroy,
> + .scan_finished = csio_scan_finished,
> + .this_id = -1,
> + .sg_tablesize = CSIO_SCSI_MAX_SGE,
> + .cmd_per_lun = CSIO_MAX_CMD_PER_LUN,
> + .use_clustering = ENABLE_CLUSTERING,
> + .shost_attrs = csio_fcoe_lport_attrs,
> + .max_sectors = CSIO_MAX_SECTOR_SIZE,
> +};
> +
> +struct scsi_host_template csio_fcoe_shost_vport_template = {
> + .module = THIS_MODULE,
> + .name = CSIO_DRV_DESC,
> + .proc_name = KBUILD_MODNAME,
> + .queuecommand = csio_queuecommand,
> + .eh_abort_handler = csio_eh_abort_handler,
> + .eh_device_reset_handler = csio_eh_lun_reset_handler,
> + .slave_alloc = csio_slave_alloc,
> + .slave_configure = csio_slave_configure,
> + .slave_destroy = csio_slave_destroy,
> + .scan_finished = csio_scan_finished,
> + .this_id = -1,
> + .sg_tablesize = CSIO_SCSI_MAX_SGE,
> + .cmd_per_lun = CSIO_MAX_CMD_PER_LUN,
> + .use_clustering = ENABLE_CLUSTERING,
> + .shost_attrs = csio_fcoe_vport_attrs,
> + .max_sectors = CSIO_MAX_SECTOR_SIZE,
> +};
> +
> +/*
> + * csio_scsi_alloc_ddp_bufs - Allocate buffers for DDP of unaligned SGLs.
> + * @scm: SCSI Module
> + * @hw: HW device.
> + * @buf_size: buffer size
> + * @num_buf : Number of buffers.
> + *
> + * This routine allocates DMA buffers required for SCSI Data xfer, if
> + * each SGL buffer for a SCSI Read request posted by SCSI midlayer are
> + * not virtually contiguous.
> + */
> +static csio_retval_t
> +csio_scsi_alloc_ddp_bufs(struct csio_scsim *scm, struct csio_hw *hw,
> + int buf_size, int num_buf)
> +{
> + int n = 0;
> + struct list_head *tmp;
> + struct csio_dma_buf *ddp_desc = NULL;
> + uint32_t unit_size = 0;
> +
> + if (!num_buf)
> + return CSIO_SUCCESS;
Just return 0..?
> +
> + if (!buf_size)
> + return CSIO_INVAL;
Just return -EINVAL..?
> +
> + INIT_LIST_HEAD(&scm->ddp_freelist);
> +
> + /* Align buf size to page size */
> + buf_size = (buf_size + PAGE_SIZE - 1) & PAGE_MASK;
> + /* Initialize dma descriptors */
> + for (n = 0; n < num_buf; n++) {
> + /* Set unit size to request size */
> + unit_size = buf_size;
> + ddp_desc = kzalloc(sizeof(struct csio_dma_buf), GFP_KERNEL);
> + if (!ddp_desc) {
> + csio_err(hw,
> + "Failed to allocate ddp descriptors,"
> + " Num allocated = %d.\n",
> + scm->stats.n_free_ddp);
> + goto no_mem;
> + }
> +
> + /* Allocate Dma buffers for DDP */
> + ddp_desc->vaddr = pci_alloc_consistent(hw->pdev, unit_size,
> + &ddp_desc->paddr);
> + if (!ddp_desc->vaddr) {
> + csio_err(hw,
> + "SCSI response DMA buffer (ddp) allocation"
> + " failed!\n");
> + kfree(ddp_desc);
> + goto no_mem;
> + }
> +
> + ddp_desc->len = unit_size;
> +
> + /* Added it to scsi ddp freelist */
> + list_add_tail(&ddp_desc->list, &scm->ddp_freelist);
> + csio_inc_stats(scm, n_free_ddp);
> + }
> +
> + return CSIO_SUCCESS;
Just return 0..?
> +no_mem:
> + /* release dma descs back to freelist and free dma memory */
> + list_for_each(tmp, &scm->ddp_freelist) {
> + ddp_desc = (struct csio_dma_buf *) tmp;
> + tmp = csio_list_prev(tmp);
> + pci_free_consistent(hw->pdev, ddp_desc->len, ddp_desc->vaddr,
> + ddp_desc->paddr);
> + list_del_init(&ddp_desc->list);
> + kfree(ddp_desc);
> + }
> + scm->stats.n_free_ddp = 0;
> +
> + return CSIO_NOMEM;
This should be just -ENOMEM..?
> +}
> +
> +/*
> + * csio_scsi_free_ddp_bufs - free DDP buffers of unaligned SGLs.
> + * @scm: SCSI Module
> + * @hw: HW device.
> + *
> + * This routine frees ddp buffers.
> + */
> +static csio_retval_t
> +csio_scsi_free_ddp_bufs(struct csio_scsim *scm, struct csio_hw *hw)
> +{
> + struct list_head *tmp;
> + struct csio_dma_buf *ddp_desc;
> +
> + /* release dma descs back to freelist and free dma memory */
> + list_for_each(tmp, &scm->ddp_freelist) {
> + ddp_desc = (struct csio_dma_buf *) tmp;
> + tmp = csio_list_prev(tmp);
> + pci_free_consistent(hw->pdev, ddp_desc->len, ddp_desc->vaddr,
> + ddp_desc->paddr);
> + list_del_init(&ddp_desc->list);
> + kfree(ddp_desc);
> + }
> + scm->stats.n_free_ddp = 0;
> +
> + return CSIO_NOMEM;
> +}
Ditto.. Just -ENOMEM..?
^ permalink raw reply
* [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Timur Tabi @ 2012-08-23 19:44 UTC (permalink / raw)
To: Andy Fleming, David Miller, ddaney.cavm, netdev,
devicetree-discuss
Add support for an MDIO bus multiplexer controlled by a simple memory-mapped
device, like an FPGA. The device must be memory-mapped and contain only
8-bit registers (which keeps things simple).
Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
to the localbus.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
.../devicetree/bindings/net/mdio-mux-mmioreg.txt | 73 ++++++++
drivers/net/phy/Kconfig | 13 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-mux-mmioreg.c | 185 ++++++++++++++++++++
4 files changed, 272 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
create mode 100644 drivers/net/phy/mdio-mux-mmioreg.c
diff --git a/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
new file mode 100644
index 0000000..251aa10
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
@@ -0,0 +1,73 @@
+Properties for an MDIO bus multiplexer controlled by a memory-mapped device
+
+This is a special case of a MDIO bus multiplexer. A memory-mapped device,
+like an FPGA, is used to control which child bus is connected. The mdio-mux
+node must be a child of the memory-mapped device. The driver currently only
+supports devices with 8-bit registers.
+
+Required properties in addition to the generic multiplexer properties:
+
+- compatible : string, must contain "mdio-mux-mmioreg"
+
+- reg : integer, contains the offset of the register that
+ controls the bus multiplexer.
+
+- mux-mask : integer, contains an 8-bit mask that specifies which
+ bits in the register control the actual bus multiplexer. The
+ 'reg' property of each child mdio-mux node must be constrained by
+ this mask.
+
+Example:
+
+The FPGA node defines a memory-mapped FPGA with a register space of 0x30 bytes.
+For the "EMI2" MDIO bus, register 9 (BRDCFG1) controls the mux on that bus.
+A bitmask of 0x6 means that bits 1 and 2 (bit 0 is lsb) are the bits on
+BRDCFG1 that control the actual mux.
+
+ /* The FPGA node */
+ fpga: board-control@3,0 {
+ compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
+ reg = <3 0 0x30>;
+
+ mdio-mux-emi2 {
+ compatible = "mdio-mux-mmioreg", "mdio-mux";
+ mdio-parent-bus = <&xmdio0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <9>; // BRDCFG1
+ mux-mask = <0x6>; // EMI2
+
+ emi2_slot1: mdio@0 { // Slot 1 XAUI (FM2)
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot1: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <4>;
+ };
+ };
+
+ emi2_slot2: mdio@2 { // Slot 2 XAUI (FM1)
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot2: ethernet-phy@4 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <0>;
+ };
+ };
+ };
+ };
+
+ /* The parent MDIO bus. */
+ xmdio0: mdio@f1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,fman-xmdio";
+ reg = <0xf1000 0x1000>;
+ interrupts = <100 1 0 0>;
+ };
+
+
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3090dc6..0268edd 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,6 +159,19 @@ config MDIO_BUS_MUX_GPIO
several child MDIO busses to a parent bus. Child bus
selection is under the control of GPIO lines.
+config MDIO_BUS_MUX_MMIOREG
+ tristate "Support for MMIO device-controlled MDIO bus multiplexers"
+ depends on OF_MDIO
+ select MDIO_BUS_MUX
+ help
+ This module provides a driver for MDIO bus multiplexers that
+ are controlled via a simple memory-mapped device, like an FPGA.
+ The multiplexer connects one of several child MDIO busses to a
+ parent bus. Child bus selection is under the control of one of
+ the FPGA's registers.
+
+ Currently, only 8-bit registers are supported.
+
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 6d2dc6c..426674d 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o
obj-$(CONFIG_AMD_PHY) += amd.o
obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o
obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o
+obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o
diff --git a/drivers/net/phy/mdio-mux-mmioreg.c b/drivers/net/phy/mdio-mux-mmioreg.c
new file mode 100644
index 0000000..e706c695
--- /dev/null
+++ b/drivers/net/phy/mdio-mux-mmioreg.c
@@ -0,0 +1,185 @@
+/*
+ * Simple memory-mapped device MDIO MUX driver
+ *
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/of_mdio.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/phy.h>
+#include <linux/mdio-mux.h>
+
+struct mdio_mux_mmioreg_state {
+ void *mux_handle;
+ phys_addr_t phys;
+ unsigned int offset;
+ uint8_t mask;
+};
+
+/*
+ * MDIO multiplexing switch function
+ *
+ * This function is called by the mdio-mux layer when it thinks the mdio bus
+ * multiplexer needs to switch.
+ *
+ * 'current_child' is the current value of the mux register (masked via
+ * s->mask).
+ *
+ * 'desired_child' is the value of the 'reg' property of the target child MDIO
+ * node.
+ *
+ * The first time this function is called, current_child == -1.
+ *
+ * If current_child == desired_child, then the mux is already set to the
+ * correct bus.
+ */
+static int mdio_mux_mmioreg_switch_fn(int current_child, int desired_child,
+ void *data)
+{
+ struct mdio_mux_mmioreg_state *s = data;
+
+ if (current_child ^ desired_child) {
+ void *p = ioremap(s->phys + s->offset, 1);
+ uint8_t x, y;
+
+ if (!p)
+ return -ENOMEM;
+
+ x = ioread8(p);
+ y = (x & ~s->mask) | desired_child;
+ if (x != y) {
+ iowrite8((x & ~s->mask) | desired_child, p);
+ pr_debug("%s: %02x -> %02x\n", __func__, x, y);
+ }
+
+ iounmap(p);
+ }
+
+ return 0;
+}
+
+static int __devinit mdio_mux_mmioreg_probe(struct platform_device *pdev)
+{
+ struct device_node *np2, *np = pdev->dev.of_node;
+ struct mdio_mux_mmioreg_state *s;
+ struct resource res;
+ const __be32 *iprop;
+ int len, ret;
+
+ dev_dbg(&pdev->dev, "probing node %s\n", np->full_name);
+
+ s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+
+ /* The MMIO device is the parent of this device */
+ np2 = of_get_parent(np);
+ if (!np2) {
+ dev_err(&pdev->dev, "could not find parent MMIO device\n");
+ return -ENODEV;
+ }
+
+ ret = of_address_to_resource(np2, 0, &res);
+ if (ret) {
+ dev_err(&pdev->dev, "could not obtain memory map for node %s\n",
+ np2->full_name);
+ return ret;
+ }
+ s->phys = res.start;
+
+ iprop = of_get_property(np, "reg", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "missing or invalid 'reg' property\n");
+ return -EINVAL;
+ }
+ s->offset = be32_to_cpup(iprop);
+ if (s->offset >= resource_size(&res)) {
+ dev_err(&pdev->dev, "'reg' property value of %u is too large\n",
+ s->offset);
+ return -EINVAL;
+ }
+
+ iprop = of_get_property(np, "mux-mask", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "missing or invalid mux-mask property\n");
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) > 255) {
+ dev_err(&pdev->dev, "only 8-bit registers are supported\n");
+ return -EINVAL;
+ }
+ s->mask = be32_to_cpup(iprop);
+
+ /*
+ * Verify that the 'reg' property of each child MDIO bus does not
+ * set any bits outside of the 'mask'.
+ */
+ for_each_available_child_of_node(np, np2) {
+ iprop = of_get_property(np2, "reg", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "mdio-mux child node %s is "
+ "missing a 'reg' property\n", np2->full_name);
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) & ~s->mask) {
+ dev_err(&pdev->dev, "mdio-mux child node %s has "
+ "a 'reg' value with unmasked bits\n",
+ np2->full_name);
+ return -ENODEV;
+ }
+ }
+
+ ret = mdio_mux_init(&pdev->dev, mdio_mux_mmioreg_switch_fn,
+ &s->mux_handle, s);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register mdio-mux bus %s\n",
+ np->full_name);
+ return ret;
+ }
+
+ pdev->dev.platform_data = s;
+
+ return 0;
+}
+
+static int __devexit mdio_mux_mmioreg_remove(struct platform_device *pdev)
+{
+ struct mdio_mux_mmioreg_state *s = dev_get_platdata(&pdev->dev);
+
+ mdio_mux_uninit(s->mux_handle);
+
+ return 0;
+}
+
+static struct of_device_id mdio_mux_mmioreg_match[] = {
+ {
+ .compatible = "mdio-mux-mmioreg",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mdio_mux_mmioreg_match);
+
+static struct platform_driver mdio_mux_mmioreg_driver = {
+ .driver = {
+ .name = "mdio-mux-mmioreg",
+ .owner = THIS_MODULE,
+ .of_match_table = mdio_mux_mmioreg_match,
+ },
+ .probe = mdio_mux_mmioreg_probe,
+ .remove = __devexit_p(mdio_mux_mmioreg_remove),
+};
+
+module_platform_driver(mdio_mux_mmioreg_driver);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("Memory-mapped device MDIO MUX driver");
+MODULE_LICENSE("GPL v2");
--
1.7.3.4
^ permalink raw reply related
* Re: [RFC PATCH bridge 2/5] bridge: Add vlan to unicast fdb entries
From: Vlad Yasevich @ 2012-08-23 19:42 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20120823123901.08b7e13e@nehalam.linuxnetplumber.net>
On 08/23/2012 03:39 PM, Stephen Hemminger wrote:
> On Thu, 23 Aug 2012 15:29:52 -0400
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>> index dd3f201..288ff10 100644
>> --- a/include/linux/if_bridge.h
>> +++ b/include/linux/if_bridge.h
>> @@ -95,6 +95,7 @@ struct __fdb_entry {
>> __u8 port_hi;
>> __u8 pad0;
>> __u16 unused;
>> +#define fdb_vid unused
>
> That is seriously ugly. Just change the structure definition
> to use the value.
Ok. I did that originally, but that made it hard to detect in
userspace. I'll think of something.
-vlad
^ permalink raw reply
* Re: [RFC PATCH bridge 0/5] Add basic VLAN support to bridges
From: Stephen Hemminger @ 2012-08-23 19:41 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1345750195-31598-1-git-send-email-vyasevic@redhat.com>
On Thu, 23 Aug 2012 15:29:50 -0400
Vlad Yasevich <vyasevic@redhat.com> wrote:
> This series of patches provides an ability to add VLAN IDs to the bridge
> ports. This is similar to what can be found in most switches. The bridge
> port may have any number of VLANs added to it including vlan 0 for untagged
> traffic. When vlans are added to the port, only traffic tagged with particular
> vlan will forwarded over this port. Additionally, vlan ids are added to FDB
> entries and become part of the lookup. This way we correctly identify the FDB
> entry.
>
> There are still pieces missing. I don't yet support adding a static fdb entry
> with a particular vlan. There is no netlink support for carrying a vlan id.
>
> I'd like to hear thoughts of whether this is usufull and something we should
> persue.
>
> The default behavior ofthe bridge is unchanged if no vlans have been
> configured.
Initial reaction is that this is a useful. You can already do the same thing
with ebtables, and ebtables allows more flexibility. But ebtables does slow
things down, and is harder to configure.
^ permalink raw reply
* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Vlad Yasevich @ 2012-08-23 19:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20120823123809.1a4adfbe@nehalam.linuxnetplumber.net>
On 08/23/2012 03:38 PM, Stephen Hemminger wrote:
> On Thu, 23 Aug 2012 15:29:54 -0400
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> Add a private ioctl to add and remove vlan configuration on bridge port.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>> include/linux/if_bridge.h | 2 +
>> net/bridge/br_if.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
>> net/bridge/br_ioctl.c | 31 ++++++++++++++++++++
>> net/bridge/br_private.h | 2 +
>> 4 files changed, 104 insertions(+), 0 deletions(-)
>
>
> Don't go down this dead end.
>
> The ioctl interface to bridge is deprecated because private ioctl's
> can not be handled when running 32 bit user space on 64 bit kernel.
>
Went down this road because is was easier to do. The plan is to do this
over netlink
-vlad
^ permalink raw reply
* Re: [RFC PATCH bridge 2/5] bridge: Add vlan to unicast fdb entries
From: Stephen Hemminger @ 2012-08-23 19:39 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1345750195-31598-3-git-send-email-vyasevic@redhat.com>
On Thu, 23 Aug 2012 15:29:52 -0400
Vlad Yasevich <vyasevic@redhat.com> wrote:
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index dd3f201..288ff10 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -95,6 +95,7 @@ struct __fdb_entry {
> __u8 port_hi;
> __u8 pad0;
> __u16 unused;
> +#define fdb_vid unused
That is seriously ugly. Just change the structure definition
to use the value.
^ permalink raw reply
* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Stephen Hemminger @ 2012-08-23 19:38 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1345750195-31598-5-git-send-email-vyasevic@redhat.com>
On Thu, 23 Aug 2012 15:29:54 -0400
Vlad Yasevich <vyasevic@redhat.com> wrote:
> Add a private ioctl to add and remove vlan configuration on bridge port.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> include/linux/if_bridge.h | 2 +
> net/bridge/br_if.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
> net/bridge/br_ioctl.c | 31 ++++++++++++++++++++
> net/bridge/br_private.h | 2 +
> 4 files changed, 104 insertions(+), 0 deletions(-)
Don't go down this dead end.
The ioctl interface to bridge is deprecated because private ioctl's
can not be handled when running 32 bit user space on 64 bit kernel.
^ permalink raw reply
* [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display VLANS
From: Vlad Yasevich @ 2012-08-23 19:29 UTC (permalink / raw)
To: netdev; +Cc: Vlad Yasevich
In-Reply-To: <1345750195-31598-1-git-send-email-vyasevic@redhat.com>
Add a binary sysfs file that will dump out vlans currently configured on the
port.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/linux/if_bridge.h | 1 +
net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
net/bridge/br_private.h | 2 ++
net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
4 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index ab750dd..d0f869b 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -20,6 +20,7 @@
#define SYSFS_BRIDGE_PORT_SUBDIR "brif"
#define SYSFS_BRIDGE_PORT_ATTR "brport"
#define SYSFS_BRIDGE_PORT_LINK "bridge"
+#define SYSFS_BRIDGE_PORT_VLANS "vlans"
#define BRCTL_VERSION 1
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 90c1038..3963748 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
return 0;
}
+size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
+ unsigned long max, unsigned long skip)
+{
+ unsigned long *map;
+ unsigned short *vid = (unsigned short *)buf;
+ unsigned short i;
+ int num = 0;
+
+ if (skip > (VLAN_N_VID+1))
+ return -EINVAL;
+
+ memset(buf, 0, max * sizeof(unsigned short));
+
+ rcu_read_lock();
+ map = rcu_dereference(p->vlan_map);
+ if (!map)
+ goto out;
+
+ for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
+ if (test_bit(i, map)) {
+ if (num > max)
+ goto out;
+
+ *vid = i-1;
+ vid++;
+ num++;
+ }
+ }
+out:
+ rcu_read_unlock();
+
+ return num*sizeof(unsigned short);
+}
+
void __net_exit br_net_exit(struct net *net)
{
struct net_device *dev;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 5639c1c..cf95cd7 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
netdev_features_t features);
extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
+extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
+ unsigned long max, unsigned long skip);
/* br_input.c */
extern int br_handle_frame_finish(struct sk_buff *skb);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 13b36bd..a81e2ef 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
};
/*
+ * Export the vlan table for a given port as a binary file.
+ * The records are unsgined shorts.
+ *
+ * Returns the number of bytes read.
+ */
+static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct net_bridge_port *p = to_brport(kobj);
+
+ return br_port_fill_vlans(p, buf,
+ count/sizeof(unsigned short),
+ off/sizeof(unsigned short));
+}
+
+static struct bin_attribute port_vlans = {
+ .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
+ .mode = S_IRUGO, },
+ .read = brport_vlans_read,
+};
+
+/*
* Add sysfs entries to ethernet device added to a bridge.
* Creates a brport subdirectory with bridge attributes.
* Puts symlink in bridge's brif subdirectory
@@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
return err;
}
+ err = sysfs_create_bin_file(&p->kobj, &port_vlans);
+ if (err) {
+ return err;
+ }
+
strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
}
--
1.7.7.6
^ permalink raw reply related
* [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Vlad Yasevich @ 2012-08-23 19:29 UTC (permalink / raw)
To: netdev; +Cc: Vlad Yasevich
In-Reply-To: <1345750195-31598-1-git-send-email-vyasevic@redhat.com>
Add a private ioctl to add and remove vlan configuration on bridge port.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/linux/if_bridge.h | 2 +
net/bridge/br_if.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
net/bridge/br_ioctl.c | 31 ++++++++++++++++++++
net/bridge/br_private.h | 2 +
4 files changed, 104 insertions(+), 0 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 288ff10..ab750dd 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -42,6 +42,8 @@
#define BRCTL_SET_PORT_PRIORITY 16
#define BRCTL_SET_PATH_COST 17
#define BRCTL_GET_FDB_ENTRIES 18
+#define BRCTL_ADD_VLAN 19
+#define BRCTL_DEL_VLAN 20
#define BR_STATE_DISABLED 0
#define BR_STATE_LISTENING 1
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index e1144e1..90c1038 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -23,6 +23,7 @@
#include <linux/if_ether.h>
#include <linux/slab.h>
#include <net/sock.h>
+#include <linux/if_vlan.h>
#include "br_private.h"
@@ -441,6 +442,74 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
return 0;
}
+/* Called with RTNL */
+int br_set_port_vlan(struct net_bridge_port *p, unsigned long vlan)
+{
+ unsigned long table_size = (VLAN_N_VID/sizeof(unsigned long)) + 1;
+ unsigned long *vid_map = NULL;
+ __u16 vid = (__u16) vlan + 1;
+
+ /* We are under lock so we can check this without rcu.
+ * The vlan map is indexed by vid+1. This way we can store
+ * vid 0 (untagged) into the map as well.
+ */
+ if (!p->vlan_map) {
+ vid_map = kzalloc(table_size, GFP_KERNEL);
+ if (!vid_map)
+ return -ENOMEM;
+
+ set_bit(vid, vid_map);
+ rcu_assign_pointer(p->vlan_map, vid_map);
+
+ } else {
+ /* Map is already allocated */
+ set_bit(vid, p->vlan_map);
+ }
+
+ return 0;
+}
+
+
+/* Called with RTNL */
+int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
+{
+ unsigned long first_bit;
+ unsigned long next_bit;
+ __u16 vid = (__u16) vlan+1;
+ unsigned long tbl_len = VLAN_N_VID+1;
+
+ if (!p->vlan_map)
+ return -EINVAL;
+
+ if (!test_bit(vlan, p->vlan_map))
+ return -EINVAL;
+
+ /* Check to see if any other vlans are in this table. If this
+ * is the last vlan, delete the whole table. If this is not the
+ * last vlan, just clear the bit.
+ */
+ first_bit = find_first_bit(p->vlan_map, tbl_len);
+ next_bit = find_next_bit(p->vlan_map, tbl_len, (tbl_len - vid));
+
+ if ((__u16)first_bit != vid || (__u16)next_bit < tbl_len) {
+ /* There are other vlans still configured. We can simply
+ * clear our bit and be safe.
+ */
+ clear_bit(vid, p->vlan_map);
+ } else {
+ /* This is the last vlan we are removing. Replace the
+ * map with a NULL pointer and free the old map
+ */
+ unsigned long *map = rcu_dereference(p->vlan_map);
+
+ rcu_assign_pointer(p->vlan_map, NULL);
+ synchronize_net();
+ kfree(map);
+ }
+
+ return 0;
+}
+
void __net_exit br_net_exit(struct net *net)
{
struct net_device *dev;
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index 7222fe1..3a5b1f9 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -289,6 +289,37 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
case BRCTL_GET_FDB_ENTRIES:
return get_fdb_entries(br, (void __user *)args[1],
args[2], args[3]);
+ case BRCTL_ADD_VLAN:
+ {
+ struct net_bridge_port *p;
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
+
+ rcu_read_lock();
+ if ((p = br_get_port(br, args[1])) == NULL) {
+ rcu_read_unlock();
+ return -EINVAL;
+ }
+ rcu_read_unlock();
+ return br_set_port_vlan(p, args[2]);
+ }
+
+ case BRCTL_DEL_VLAN:
+ {
+ struct net_bridge_port *p;
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
+
+ rcu_read_lock();
+ if ((p = br_get_port(br, args[1])) == NULL) {
+ rcu_read_unlock();
+ return -EINVAL;
+ }
+ rcu_read_unlock();
+ br_set_port_vlan(p, args[2]);
+ }
}
return -EOPNOTSUPP;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b6c56ab..5639c1c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -402,6 +402,8 @@ extern int br_del_if(struct net_bridge *br,
extern int br_min_mtu(const struct net_bridge *br);
extern netdev_features_t br_features_recompute(struct net_bridge *br,
netdev_features_t features);
+extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
+extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
/* br_input.c */
extern int br_handle_frame_finish(struct sk_buff *skb);
--
1.7.7.6
^ 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