* 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
* [RFC PATCH bridge 3/5] bridge: Add vlan id to multicast groups
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 vlan_id to multicasts groups so that we know which vlan each group belongs
to and can correctly forward to appropriate vlan.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_multicast.c | 64 +++++++++++++++++++++++++++++++--------------
net/bridge/br_private.h | 1 +
2 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2417434..2976a2b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -51,6 +51,8 @@ static inline int br_ip_equal(const struct br_ip *a, const struct br_ip *b)
{
if (a->proto != b->proto)
return 0;
+ if (a->vid != b->vid)
+ return 0;
switch (a->proto) {
case htons(ETH_P_IP):
return a->u.ip4 == b->u.ip4;
@@ -62,16 +64,19 @@ static inline int br_ip_equal(const struct br_ip *a, const struct br_ip *b)
return 0;
}
-static inline int __br_ip4_hash(struct net_bridge_mdb_htable *mdb, __be32 ip)
+static inline int __br_ip4_hash(struct net_bridge_mdb_htable *mdb, __be32 ip,
+ __u16 vid)
{
- return jhash_1word(mdb->secret, (__force u32)ip) & (mdb->max - 1);
+ return jhash_2words((__force u32)ip, vid, mdb->secret) & (mdb->max - 1);
}
#if IS_ENABLED(CONFIG_IPV6)
static inline int __br_ip6_hash(struct net_bridge_mdb_htable *mdb,
- const struct in6_addr *ip)
+ const struct in6_addr *ip,
+ __u16 vid)
{
- return jhash2((__force u32 *)ip->s6_addr32, 4, mdb->secret) & (mdb->max - 1);
+ u32 addr = *(__force u32 *)ip->s6_addr32;
+ return jhash_2words(addr, vid, mdb->secret) & (mdb->max - 1);
}
#endif
@@ -80,10 +85,10 @@ static inline int br_ip_hash(struct net_bridge_mdb_htable *mdb,
{
switch (ip->proto) {
case htons(ETH_P_IP):
- return __br_ip4_hash(mdb, ip->u.ip4);
+ return __br_ip4_hash(mdb, ip->u.ip4, ip->vid);
#if IS_ENABLED(CONFIG_IPV6)
case htons(ETH_P_IPV6):
- return __br_ip6_hash(mdb, &ip->u.ip6);
+ return __br_ip6_hash(mdb, &ip->u.ip6, ip->vid);
#endif
}
return 0;
@@ -113,24 +118,27 @@ static struct net_bridge_mdb_entry *br_mdb_ip_get(
}
static struct net_bridge_mdb_entry *br_mdb_ip4_get(
- struct net_bridge_mdb_htable *mdb, __be32 dst)
+ struct net_bridge_mdb_htable *mdb, __be32 dst, __u16 vlan_tci)
{
struct br_ip br_dst;
br_dst.u.ip4 = dst;
br_dst.proto = htons(ETH_P_IP);
+ br_dst.vid = (vlan_tci & VLAN_VID_MASK);
return br_mdb_ip_get(mdb, &br_dst);
}
#if IS_ENABLED(CONFIG_IPV6)
static struct net_bridge_mdb_entry *br_mdb_ip6_get(
- struct net_bridge_mdb_htable *mdb, const struct in6_addr *dst)
+ struct net_bridge_mdb_htable *mdb, const struct in6_addr *dst,
+ __u16 vlan_tci)
{
struct br_ip br_dst;
br_dst.u.ip6 = *dst;
br_dst.proto = htons(ETH_P_IPV6);
+ br_dst.vid = vlan_tci & VLAN_VID_MASK;
return br_mdb_ip_get(mdb, &br_dst);
}
@@ -692,7 +700,8 @@ err:
static int br_ip4_multicast_add_group(struct net_bridge *br,
struct net_bridge_port *port,
- __be32 group)
+ __be32 group,
+ __u16 vlan_tci)
{
struct br_ip br_group;
@@ -701,6 +710,7 @@ static int br_ip4_multicast_add_group(struct net_bridge *br,
br_group.u.ip4 = group;
br_group.proto = htons(ETH_P_IP);
+ br_group.vid = vlan_tci & VLAN_VID_MASK;
return br_multicast_add_group(br, port, &br_group);
}
@@ -708,7 +718,8 @@ static int br_ip4_multicast_add_group(struct net_bridge *br,
#if IS_ENABLED(CONFIG_IPV6)
static int br_ip6_multicast_add_group(struct net_bridge *br,
struct net_bridge_port *port,
- const struct in6_addr *group)
+ const struct in6_addr *group,
+ __u16 vlan_tci)
{
struct br_ip br_group;
@@ -717,6 +728,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
br_group.u.ip6 = *group;
br_group.proto = htons(ETH_P_IPV6);
+ br_group.vid = vlan_tci & VLAN_VID_MASK;
return br_multicast_add_group(br, port, &br_group);
}
@@ -928,7 +940,8 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
continue;
}
- err = br_ip4_multicast_add_group(br, port, group);
+ err = br_ip4_multicast_add_group(br, port, group,
+ skb->vlan_tci);
if (err)
break;
}
@@ -988,7 +1001,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
continue;
}
- err = br_ip6_multicast_add_group(br, port, &grec->grec_mca);
+ err = br_ip6_multicast_add_group(br, port, &grec->grec_mca,
+ skb->vlan_tci);
if (!err)
break;
}
@@ -1106,7 +1120,8 @@ static int br_ip4_multicast_query(struct net_bridge *br,
if (!group)
goto out;
- mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group);
+ mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group,
+ skb->vlan_tci);
if (!mp)
goto out;
@@ -1178,7 +1193,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
if (!group)
goto out;
- mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group);
+ mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group,
+ skb->vlan_tci);
if (!mp)
goto out;
@@ -1262,7 +1278,8 @@ out:
static void br_ip4_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
- __be32 group)
+ __be32 group,
+ __u16 vlan_tci)
{
struct br_ip br_group;
@@ -1271,6 +1288,7 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
br_group.u.ip4 = group;
br_group.proto = htons(ETH_P_IP);
+ br_group.vid = (vlan_tci & VLAN_VID_MASK);
br_multicast_leave_group(br, port, &br_group);
}
@@ -1278,7 +1296,8 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
#if IS_ENABLED(CONFIG_IPV6)
static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
- const struct in6_addr *group)
+ const struct in6_addr *group,
+ __u16 vlan_tci)
{
struct br_ip br_group;
@@ -1287,6 +1306,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
br_group.u.ip6 = *group;
br_group.proto = htons(ETH_P_IPV6);
+ br_group.vid = (vlan_tci & VLAN_VID_MASK);
br_multicast_leave_group(br, port, &br_group);
}
@@ -1369,7 +1389,8 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip4_multicast_add_group(br, port, ih->group);
+ err = br_ip4_multicast_add_group(br, port, ih->group,
+ skb2->vlan_tci);
break;
case IGMPV3_HOST_MEMBERSHIP_REPORT:
err = br_ip4_multicast_igmp3_report(br, port, skb2);
@@ -1378,7 +1399,8 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
err = br_ip4_multicast_query(br, port, skb2);
break;
case IGMP_HOST_LEAVE_MESSAGE:
- br_ip4_multicast_leave_group(br, port, ih->group);
+ br_ip4_multicast_leave_group(br, port, ih->group,
+ skb2->vlan_tci);
break;
}
@@ -1498,7 +1520,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
}
mld = (struct mld_msg *)skb_transport_header(skb2);
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip6_multicast_add_group(br, port, &mld->mld_mca);
+ err = br_ip6_multicast_add_group(br, port, &mld->mld_mca,
+ skb2->vlan_tci);
break;
}
case ICMPV6_MLD2_REPORT:
@@ -1515,7 +1538,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
goto out;
}
mld = (struct mld_msg *)skb_transport_header(skb2);
- br_ip6_multicast_leave_group(br, port, &mld->mld_mca);
+ br_ip6_multicast_leave_group(br, port, &mld->mld_mca,
+ skb2->vlan_tci);
}
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 921b927..b6c56ab 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -61,6 +61,7 @@ struct br_ip
#endif
} u;
__be16 proto;
+ __u16 vid;
};
struct net_bridge_fdb_entry
--
1.7.7.6
^ permalink raw reply related
* [RFC PATCH bridge 2/5] bridge: Add vlan to unicast fdb entries
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>
This patch adds vlan to unicast fdb entries that are created for
learned addresses (not the manually configured ones). It adds
vlan id into the hash mix and uses vlan as an addditional parameter
for an entry match.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/linux/if_bridge.h | 1 +
net/bridge/br_device.c | 2 +-
net/bridge/br_fdb.c | 71 ++++++++++++++++++++++++++------------------
net/bridge/br_input.c | 14 ++++++---
net/bridge/br_private.h | 7 +++-
5 files changed, 58 insertions(+), 37 deletions(-)
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
};
#ifdef __KERNEL__
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 3334845..e321b9d 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -66,7 +66,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
br_multicast_deliver(mdst, skb);
else
br_flood_deliver(br, skb);
- } else if ((dst = __br_fdb_get(br, dest)) != NULL)
+ } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) != NULL)
br_deliver(dst->dst, skb);
else
br_flood_deliver(br, skb);
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index d21f323..478ca1d 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -23,6 +23,7 @@
#include <linux/slab.h>
#include <linux/atomic.h>
#include <asm/unaligned.h>
+#include <linux/if_vlan.h>
#include "br_private.h"
static struct kmem_cache *br_fdb_cache __read_mostly;
@@ -67,11 +68,12 @@ static inline int has_expired(const struct net_bridge *br,
time_before_eq(fdb->updated + hold_time(br), jiffies);
}
-static inline int br_mac_hash(const unsigned char *mac)
+static inline int br_mac_hash(const unsigned char *mac, __u16 vlan_tci)
{
- /* use 1 byte of OUI cnd 3 bytes of NIC */
+ /* use 1 byte of OUI and 3 bytes of NIC */
u32 key = get_unaligned((u32 *)(mac + 2));
- return jhash_1word(key, fdb_salt) & (BR_HASH_SIZE - 1);
+ return jhash_2words(key, (vlan_tci & VLAN_VID_MASK),
+ fdb_salt) & (BR_HASH_SIZE - 1);
}
static void fdb_rcu_free(struct rcu_head *head)
@@ -132,7 +134,7 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
struct net_bridge_fdb_entry *f;
/* If old entry was unassociated with any port, then delete it. */
- f = __br_fdb_get(br, br->dev->dev_addr);
+ f = __br_fdb_get(br, br->dev->dev_addr, 0);
if (f && f->is_local && !f->dst)
fdb_delete(br, f);
@@ -231,13 +233,16 @@ void br_fdb_delete_by_port(struct net_bridge *br,
/* No locking or refcounting, assumes caller has rcu_read_lock */
struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vlan_tci)
{
struct hlist_node *h;
struct net_bridge_fdb_entry *fdb;
- hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
- if (ether_addr_equal(fdb->addr.addr, addr)) {
+ hlist_for_each_entry_rcu(fdb, h,
+ &br->hash[br_mac_hash(addr, vlan_tci)], hlist) {
+ if (ether_addr_equal(fdb->addr.addr, addr) &&
+ fdb->vlan_id == (vlan_tci & VLAN_VID_MASK) ) {
if (unlikely(has_expired(br, fdb)))
break;
return fdb;
@@ -261,7 +266,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
if (!port)
ret = 0;
else {
- fdb = __br_fdb_get(port->br, addr);
+ fdb = __br_fdb_get(port->br, addr, 0);
ret = fdb && fdb->dst && fdb->dst->dev != dev &&
fdb->dst->state == BR_STATE_FORWARDING;
}
@@ -313,6 +318,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
fe->is_local = f->is_local;
if (!f->is_static)
fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->updated);
+ fe->fdb_vid = f->vlan_id;
++fe;
++num;
}
@@ -325,26 +331,30 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
}
static struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vlan_tci)
{
struct hlist_node *h;
struct net_bridge_fdb_entry *fdb;
hlist_for_each_entry(fdb, h, head, hlist) {
- if (ether_addr_equal(fdb->addr.addr, addr))
+ if (ether_addr_equal(fdb->addr.addr, addr) &&
+ fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
return fdb;
}
return NULL;
}
static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vlan_tci)
{
struct hlist_node *h;
struct net_bridge_fdb_entry *fdb;
hlist_for_each_entry_rcu(fdb, h, head, hlist) {
- if (ether_addr_equal(fdb->addr.addr, addr))
+ if (ether_addr_equal(fdb->addr.addr, addr) &&
+ fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
return fdb;
}
return NULL;
@@ -352,7 +362,8 @@ static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
struct net_bridge_port *source,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vlan_tci)
{
struct net_bridge_fdb_entry *fdb;
@@ -360,6 +371,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
if (fdb) {
memcpy(fdb->addr.addr, addr, ETH_ALEN);
fdb->dst = source;
+ fdb->vlan_id = (vlan_tci & VLAN_VID_MASK);
fdb->is_local = 0;
fdb->is_static = 0;
fdb->updated = fdb->used = jiffies;
@@ -371,13 +383,13 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
struct net_bridge_fdb_entry *fdb;
if (!is_valid_ether_addr(addr))
return -EINVAL;
- fdb = fdb_find(head, addr);
+ fdb = fdb_find(head, addr, 0);
if (fdb) {
/* it is okay to have multiple ports with same
* address, just use the first one.
@@ -390,7 +402,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
fdb_delete(br, fdb);
}
- fdb = fdb_create(head, source, addr);
+ fdb = fdb_create(head, source, addr, 0);
if (!fdb)
return -ENOMEM;
@@ -412,9 +424,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
}
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr)
+ const unsigned char *addr, __u16 vlan_tci)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
struct net_bridge_fdb_entry *fdb;
/* some users want to always flood. */
@@ -426,7 +438,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
source->state == BR_STATE_FORWARDING))
return;
- fdb = fdb_find_rcu(head, addr);
+ fdb = fdb_find_rcu(head, addr, vlan_tci);
if (likely(fdb)) {
/* attempt to update an entry for a local interface */
if (unlikely(fdb->is_local)) {
@@ -441,8 +453,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
}
} else {
spin_lock(&br->hash_lock);
- if (likely(!fdb_find(head, addr))) {
- fdb = fdb_create(head, source, addr);
+ if (likely(!fdb_find(head, addr, vlan_tci))) {
+ fdb = fdb_create(head, source, addr, vlan_tci);
if (fdb)
fdb_notify(br, fdb, RTM_NEWNEIGH);
}
@@ -571,18 +583,18 @@ out:
/* Update (create or replace) forwarding database entry */
static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
- __u16 state, __u16 flags)
+ __u16 state, __u16 flags, __u16 vlan_tci)
{
struct net_bridge *br = source->br;
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
struct net_bridge_fdb_entry *fdb;
- fdb = fdb_find(head, addr);
+ fdb = fdb_find(head, addr, vlan_tci);
if (fdb == NULL) {
if (!(flags & NLM_F_CREATE))
return -ENOENT;
- fdb = fdb_create(head, source, addr);
+ fdb = fdb_create(head, source, addr, vlan_tci);
if (!fdb)
return -ENOMEM;
fdb_notify(br, fdb, RTM_NEWNEIGH);
@@ -628,11 +640,12 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
if (ndm->ndm_flags & NTF_USE) {
rcu_read_lock();
- br_fdb_update(p->br, p, addr);
+ br_fdb_update(p->br, p, addr, 0);
rcu_read_unlock();
} else {
spin_lock_bh(&p->br->hash_lock);
- err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags);
+ err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags,
+ 0);
spin_unlock_bh(&p->br->hash_lock);
}
@@ -642,10 +655,10 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
static int fdb_delete_by_addr(struct net_bridge_port *p, u8 *addr)
{
struct net_bridge *br = p->br;
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
struct net_bridge_fdb_entry *fdb;
- fdb = fdb_find(head, addr);
+ fdb = fdb_find(head, addr, 0);
if (!fdb)
return -ENOENT;
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index b7ca3b3..a4579ee 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -62,12 +62,15 @@ int br_handle_frame_finish(struct sk_buff *skb)
* only traffic matching the VLAN filter.
*/
vlan_map = rcu_dereference(p->vlan_map);
- if (vlan_map && !test_bit((skb->vlan_tci & VLAN_VID_MASK), vlan_map))
- goto drop;
+ if (vlan_map && vlan_tx_tag_present(skb)) {
+ unsigned short vid = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
+ if (!test_bit(vid+1, vlan_map))
+ goto drop;
+ }
/* insert into forwarding database after filtering to avoid spoofing */
br = p->br;
- br_fdb_update(br, p, eth_hdr(skb)->h_source);
+ br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) &&
br_multicast_rcv(br, p, skb))
@@ -102,7 +105,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
skb2 = skb;
br->dev->stats.multicast++;
- } else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+ } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) &&
+ dst->is_local) {
skb2 = skb;
/* Do not forward the packet since it's local. */
skb = NULL;
@@ -131,7 +135,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
{
struct net_bridge_port *p = br_port_get_rcu(skb->dev);
- br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+ br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
return 0; /* process further */
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 8da90e8..921b927 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -72,6 +72,7 @@ struct net_bridge_fdb_entry
unsigned long updated;
unsigned long used;
mac_addr addr;
+ __u16 vlan_id;
unsigned char is_local;
unsigned char is_static;
};
@@ -352,7 +353,8 @@ extern void br_fdb_cleanup(unsigned long arg);
extern void br_fdb_delete_by_port(struct net_bridge *br,
const struct net_bridge_port *p, int do_all);
extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
- const unsigned char *addr);
+ const unsigned char *addr,
+ __u16 vlan_tci);
extern int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
unsigned long count, unsigned long off);
@@ -361,7 +363,8 @@ extern int br_fdb_insert(struct net_bridge *br,
const unsigned char *addr);
extern void br_fdb_update(struct net_bridge *br,
struct net_bridge_port *source,
- const unsigned char *addr);
+ const unsigned char *addr,
+ __u16 vlan_tci);
extern int br_fdb_delete(struct ndmsg *ndm,
struct net_device *dev,
--
1.7.7.6
^ permalink raw reply related
* [RFC PATCH bridge 1/5] bridge: Add vlan check to forwarding path
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>
When forwarding packets make sure vlan matches any configured vlan for
the port.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_forward.c | 17 ++++++++++++++++-
net/bridge/br_input.c | 8 ++++++++
net/bridge/br_private.h | 2 ++
3 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index e9466d4..ab81954 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -26,11 +26,26 @@ static int deliver_clone(const struct net_bridge_port *prev,
void (*__packet_hook)(const struct net_bridge_port *p,
struct sk_buff *skb));
-/* Don't forward packets to originating port or forwarding diasabled */
+/* check to see that the vlan is allowed to be forwarded on this interface */
+static inline int vlan_match(const struct net_bridge_port *p,
+ const struct sk_buff *skb)
+{
+ unsigned long *vlan_map = rcu_dereference(p->vlan_map);
+ unsigned short vid = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
+
+ /* The map keeps the vlans off by 1 so adjust for that */
+ return (vlan_map && vlan_tx_tag_present(skb) &&
+ test_bit(vid+1, vlan_map));
+}
+
+/* Don't forward packets to originating port or forwarding diasabled.
+ */
static inline int should_deliver(const struct net_bridge_port *p,
const struct sk_buff *skb)
{
+
return (((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
+ vlan_match(p, skb) &&
p->state == BR_STATE_FORWARDING);
}
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 76f15fd..b7ca3b3 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -53,10 +53,18 @@ int br_handle_frame_finish(struct sk_buff *skb)
struct net_bridge_fdb_entry *dst;
struct net_bridge_mdb_entry *mdst;
struct sk_buff *skb2;
+ unsigned long *vlan_map;
if (!p || p->state == BR_STATE_DISABLED)
goto drop;
+ /* If VLAN filter is configured on the port, make sure we accept
+ * only traffic matching the VLAN filter.
+ */
+ vlan_map = rcu_dereference(p->vlan_map);
+ if (vlan_map && !test_bit((skb->vlan_tci & VLAN_VID_MASK), vlan_map))
+ goto drop;
+
/* insert into forwarding database after filtering to avoid spoofing */
br = p->br;
br_fdb_update(br, p, eth_hdr(skb)->h_source);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a768b24..8da90e8 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -152,6 +152,8 @@ struct net_bridge_port
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *np;
#endif
+ unsigned long __rcu *vlan_map;
+
};
#define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
--
1.7.7.6
^ permalink raw reply related
* [RFC PATCH bridge 0/5] Add basic VLAN support to bridges
From: Vlad Yasevich @ 2012-08-23 19:29 UTC (permalink / raw)
To: netdev; +Cc: Vlad Yasevich
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.
Vlad Yasevich (5):
bridge: Add vlan check to forwarding path
bridge: Add vlan to unicast fdb entries
bridge: Add vlan id to multicast groups
bridge: Add private ioctls to configure vlans on bridge ports
bridge: Add sysfs interface to display VLANS
include/linux/if_bridge.h | 4 ++
net/bridge/br_device.c | 2 +-
net/bridge/br_fdb.c | 71 ++++++++++++++++++-------------
net/bridge/br_forward.c | 17 +++++++-
net/bridge/br_if.c | 103 +++++++++++++++++++++++++++++++++++++++++++++
net/bridge/br_input.c | 18 +++++++-
net/bridge/br_ioctl.c | 31 +++++++++++++
net/bridge/br_multicast.c | 64 +++++++++++++++++++---------
net/bridge/br_private.h | 14 +++++-
net/bridge/br_sysfs_if.c | 28 ++++++++++++
10 files changed, 296 insertions(+), 56 deletions(-)
--
1.7.7.6
^ permalink raw reply
* RE: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Vick, Matthew @ 2012-08-23 18:58 UTC (permalink / raw)
To: Keller, Jacob E, Richard Cochran
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <02874ECE860811409154E81DA85FBB5807857FFC@ORSMSX105.amr.corp.intel.com>
> -----Original Message-----
> From: Keller, Jacob E
> Sent: Thursday, August 23, 2012 11:48 AM
> To: Vick, Matthew; Richard Cochran
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: RE: [net-next 11/13] igb: Update PTP function names/variables
> and locations.
>
> [...]
>
> The history isn't lost, it is just obscured. I agree if the change is
> necessary it should be done. I think we all disagree on what is
> necessary. I would prefer function names to match. Richard has a point
> regarding the time stamping all packets, however again the ioctl
> turning that on tends to be quite PTP centric already. I think that
> value isn't necessarily added due to the current coupling of the
> features. It is partially coupled by the hardware anyways.
>
> Effectively with the PTP framework disabled you have a half useful
> feature that is enabled by a strange ioctl that has a lot of PTP
> specific names, and doesn't always do what you want.
>
> I am not sure how much work would be required to get to a state where
> the separation of function makes sense.
>
> - Jake
Fair point about the history.
Since these functions are tightly coupled with PTP, it makes sense to call them that for now. Long-term, I completely agree with Richard--once the function is independent of PTP, we should give it a generic name.
Cheers,
Matthew
^ permalink raw reply
* RE: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Keller, Jacob E @ 2012-08-23 18:48 UTC (permalink / raw)
To: Vick, Matthew, Richard Cochran
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <06DFBC1E25D8024DB214DC7F41A3CD34488DD32D@ORSMSX101.amr.corp.intel.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Vick, Matthew
> Sent: Thursday, August 23, 2012 11:35 AM
> To: Richard Cochran
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: RE: [net-next 11/13] igb: Update PTP function names/variables and
> locations.
>
> > -----Original Message-----
> > From: Richard Cochran [mailto:richardcochran@gmail.com]
> > Sent: Thursday, August 23, 2012 10:54 AM
> > To: Vick, Matthew
> > Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org;
> > gospo@redhat.com; sassmann@redhat.com
> > Subject: Re: [net-next 11/13] igb: Update PTP function names/variables
> > and locations.
> >
> > On Thu, Aug 23, 2012 at 04:22:02PM +0000, Vick, Matthew wrote:
> >
> > > > > #ifdef CONFIG_IGB_PTP
> > > > > -static int igb_ethtool_get_ts_info(struct net_device *dev,
> > > > > +static int igb_get_ts_info(struct net_device *dev,
> > > >
> > > > I like the old name better.
> > >
> > > The old name is out of the coding style of igb. Every other function
> > is igb_get_* or igb_set_*, with the exception of igb_ethtool_begin and
> > igb_ethtool_complete. Are you suggesting we add _ethtool to every
> > ethtool function in igb?
> >
> > No, just leave the names alone, and keep the functions where they are.
> > It is just churn.
> >
> > One of the most useful ways to understand code (at least for me) is to
> > use git blame. It tells you when code was added, what the reason was,
> > and how the change looks in context. By moving and renaming willy
> > nilly, you are obscuring this valuable information.
>
> Repairing is not churn. I think it's better to make the code clearer than
> require developers use git blame to figure out who named a function in a
> silly way. I agree, it kind of stinks to lose that history, but I'd rather
> not skip making the right change because of git blame concerns.
The history isn't lost, it is just obscured. I agree if the change is necessary it should be done. I think we all disagree on what is necessary. I would prefer function names to match. Richard has a point regarding the time stamping all packets, however again the ioctl turning that on tends to be quite PTP centric already. I think that value isn't necessarily added due to the current coupling of the features. It is partially coupled by the hardware anyways.
Effectively with the PTP framework disabled you have a half useful feature that is enabled by a strange ioctl that has a lot of PTP specific names, and doesn't always do what you want.
I am not sure how much work would be required to get to a state where the separation of function makes sense.
- Jake
>
> >
> > [...]
> >
> > >
> > > Which, this function calls igb_systim_to_hwtstamp anyway, which is
> > > global.
> >
> > So how does calling two global functions in series improve performance?
>
> It isn't calling two global functions. It's calling a global function that
> calls a static function.
>
> > > Also, in a follow-on patch I have coming, igb_ptp_tx_hwtstamp won't
> > > even be called in clean_tx_irq, FWIW.
> >
> > If this is part of some larger plan, then it would help to see that
> > plan.
>
> Definitely fair enough. I will work with Jeff to try and push the whole
> series next time (at the very least, I'd like to re-spin the series for
> the ethtool query patch).
>
> > > > > /**
> > > > > * igb_clean_tx_irq - Reclaim resources after transmit completes
> > > > > * @q_vector: pointer to q_vector containing needed info @@
> > > > > -5827,7
> > > > > +5796,7 @@ static bool igb_clean_tx_irq(struct igb_q_vector
> > > > *q_vector)
> > > > >
> > > > > #ifdef CONFIG_IGB_PTP
> > > > > /* retrieve hardware timestamp */
> > > > > - igb_tx_hwtstamp(q_vector, tx_buffer);
> > > > > + igb_ptp_tx_hwtstamp(q_vector, tx_buffer);
> > > >
> > > > This name stinks, too. You know that you can have time stamping
> > > > all by itself, right? It is logically separate from the ptp clock
> > stuff.
> > > >
> > > > This patch doesn't really improve the driver at all, IMHO.
> > > >
> > > > Thanks,
> > > > Richard
> > >
> > > Yes, I'm aware. But, as it stands today, we don't use it for
> > > anything
> > else. If the function is feature specific, then we should be calling
> > it out as such.
> >
> > Right now the time stamping is being equated with the clock functions,
> > but it really should be decoupled. The 82580 can time stamp every
> > received packet, which can be interesting for performance monitoring,
> > even without PTP (and adding *that* would be a useful change).
>
> As Jake said, there's no support for hardware timestamping without PTP
> today. As such, if the function is only useful for that feature, then I
> think it's less ambiguous to leave it the way I've coded it today. If we
> de-couple hardware timestamping and clock synchronization, which I
> wouldn't be opposed to, then the function name can reflect becoming more
> generic. As it stands, I'd rather not call something generic when it
> isn't. I think it's misleading, personally.
>
> > > I'm sorry you feel like this patch doesn't improve the driver. The
> > goal is code cleanup and consistency, both of which I consider to be
> > driver improvements and is why I made the patches.
> >
> > But the code wasn't dirty in the first place. It doesn't need this
> > "cleaning." This series undoes the inline-able functions for no good
> > reason. As far as ixgbe goes, this driver came first, so you might as
> > well be making *that* driver consistent with this one.
> >
> > Thanks,
> > Richard
>
> Actually, yes, the code *was* dirty, because it breaks the coding style of
> the rest of the driver and isn't as self-documenting as it could be, and
> the inline-able functions you claim I'm destroying called global functions
> anyway. Also, just because igb came first doesn't mean we should ignore
> porting better or clearer solutions from later drivers. If something is
> better or clearer, I think we should use be using it.
>
> Cheers,
> Matthew
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Vick, Matthew @ 2012-08-23 18:44 UTC (permalink / raw)
To: Richard Cochran, Keller, Jacob E
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <20120823181117.GD2192@netboy.at.omicron.at>
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Thursday, August 23, 2012 11:11 AM
> To: Keller, Jacob E
> Cc: Vick, Matthew; Kirsher, Jeffrey T; davem@davemloft.net;
> netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 11/13] igb: Update PTP function names/variables
> and locations.
>
> [...]
>
> Relative, high resolution time stamps can be interesting all by
> themselves. That is why wireshark has a whole menu of timing choices
> including relative since start, inter-packet, and so on.
>
> [...]
>
> Yes, I agree that igb is a bit oddly synced WRT clock and time
> stamping. I would welcome a change to let it have HW time stamping as
> an independent feature.
>
> Thanks,
> Richard
Isn't this discussion creeping outside the scope of this patch? My aim here with this patch is to help tidy up PTP code as it is today. As it is today, igb has no support for hardware timestamping without PTP. If you or someone else writes a follow-on patch to add hardware timestamping without PTP, then they can move and rename the functions accordingly (and I'd personally really appreciate it if they did rename it, since a generic function should be named like a generic function and a PTP function should be named like a PTP function).
Cheers,
Matthew
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox