* Fwd: [PATCH] forcedeth: reconfigure multicast packet filter only when needed [not found] <20100812115430.2b5d8683@starbug.prg01.itonis.net> @ 2010-10-09 11:26 ` Jindřich Makovička 2010-10-09 18:32 ` Stephen Hemminger 0 siblings, 1 reply; 3+ messages in thread From: Jindřich Makovička @ 2010-10-09 11:26 UTC (permalink / raw) To: linux-kernel, netdev; +Cc: davem, shemminger, aabdulla, ditto, makovick [-- Attachment #1: Type: text/plain, Size: 824 bytes --] Currently, the forcedeth driver reconfigures the packet filter every time a multicast stream is (un-)subscribed. As the receiving has to be stopped and started in this case, any multicast subscription can cause packet loss, allowing userspace applications to disrupt incoming traffic. With the following patch, nv_set_multicast first checks the cached state of the packet filter, and skips the reconfiguration if the state does not change. With the default settings, this can reduce some useless reconfiguration attempts. When switched to promiscuous mode, all reconfigurations are skipped with the patch, which can be used to mitigate packet loss problems when receiving and re-subscribing many multicasts simultaneously on a single machine. Signed-off-by: Jindrich Makovicka <makovick@gmail•com> -- Jindrich Makovicka [-- Attachment #2: forcedeth.c.diff --] [-- Type: text/x-patch, Size: 2212 bytes --] --- forcedeth.c.orig 2010-04-26 16:48:30.000000000 +0200 +++ forcedeth.c 2010-05-21 13:22:25.705907294 +0200 @@ -837,6 +837,11 @@ char name_rx[IFNAMSIZ + 3]; /* -rx */ char name_tx[IFNAMSIZ + 3]; /* -tx */ char name_other[IFNAMSIZ + 6]; /* -other */ + + /* current packet filter state */ + u32 cur_pff; + u32 cur_addr[2]; + u32 cur_mask[2]; }; /* @@ -3128,17 +3133,28 @@ } addr[0] |= NVREG_MCASTADDRA_FORCE; pff |= NVREG_PFF_ALWAYS; - spin_lock_irq(&np->lock); - nv_stop_rx(dev); - writel(addr[0], base + NvRegMulticastAddrA); - writel(addr[1], base + NvRegMulticastAddrB); - writel(mask[0], base + NvRegMulticastMaskA); - writel(mask[1], base + NvRegMulticastMaskB); - writel(pff, base + NvRegPacketFilterFlags); - dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n", - dev->name); - nv_start_rx(dev); - spin_unlock_irq(&np->lock); + if (np->cur_pff != (pff & ~NVREG_PFF_PAUSE_RX) + || memcmp(np->cur_addr, addr, sizeof(np->cur_addr)) != 0 + || memcmp(np->cur_mask, mask, sizeof(np->cur_mask)) != 0) + { + dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n", + dev->name); + spin_lock_irq(&np->lock); + nv_stop_rx(dev); + writel(addr[0], base + NvRegMulticastAddrA); + writel(addr[1], base + NvRegMulticastAddrB); + writel(mask[0], base + NvRegMulticastMaskA); + writel(mask[1], base + NvRegMulticastMaskB); + writel(pff, base + NvRegPacketFilterFlags); + nv_start_rx(dev); + spin_unlock_irq(&np->lock); + memcpy(np->cur_addr, addr, sizeof(np->cur_addr)); + memcpy(np->cur_mask, mask, sizeof(np->cur_mask)); + np->cur_pff = pff & ~NVREG_PFF_PAUSE_RX; + } else { + dprintk(KERN_INFO "%s: pff state unchanged - skipping reconfiguration.\n", + dev->name); + } } static void nv_update_pause(struct net_device *dev, u32 pause_flags) @@ -5369,6 +5385,12 @@ writel(NVREG_MCASTMASKB_NONE, base + NvRegMulticastMaskB); writel(0, base + NvRegPacketFilterFlags); + np->cur_pff = 0; + np->cur_addr[0] = NVREG_MCASTADDRA_FORCE; + np->cur_addr[1] = 0; + np->cur_mask[0] = NVREG_MCASTMASKA_NONE; + np->cur_mask[1] = NVREG_MCASTMASKB_NONE; + writel(0, base + NvRegTransmitterControl); writel(0, base + NvRegReceiverControl); ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] forcedeth: reconfigure multicast packet filter only when needed 2010-10-09 11:26 ` Fwd: [PATCH] forcedeth: reconfigure multicast packet filter only when needed Jindřich Makovička @ 2010-10-09 18:32 ` Stephen Hemminger 2010-10-09 20:46 ` Jindřich Makovička 0 siblings, 1 reply; 3+ messages in thread From: Stephen Hemminger @ 2010-10-09 18:32 UTC (permalink / raw) To: Jindřich Makovička; +Cc: linux-kernel, netdev, davem, aabdulla, ditto On Sat, 9 Oct 2010 13:26:05 +0200 Jindřich Makovička <makovick@gmail•com> wrote: > + > + /* current packet filter state */ > + u32 cur_pff; > + u32 cur_addr[2]; > + u32 cur_mask[2]; > }; No big deal, but couldn't you just put those temporary variables on the stack. and reread the current value before stopping. -- ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] forcedeth: reconfigure multicast packet filter only when needed 2010-10-09 18:32 ` Stephen Hemminger @ 2010-10-09 20:46 ` Jindřich Makovička 0 siblings, 0 replies; 3+ messages in thread From: Jindřich Makovička @ 2010-10-09 20:46 UTC (permalink / raw) To: Stephen Hemminger; +Cc: linux-kernel, netdev, davem, aabdulla, mditto [-- Attachment #1: Type: text/plain, Size: 661 bytes --] 2010/10/9 Stephen Hemminger <shemminger@vyatta•com>: > On Sat, 9 Oct 2010 13:26:05 +0200 > Jindřich Makovička <makovick@gmail•com> wrote: > >> + >> + /* current packet filter state */ >> + u32 cur_pff; >> + u32 cur_addr[2]; >> + u32 cur_mask[2]; >> }; > > No big deal, but couldn't you just put those temporary variables > on the stack. and reread the current value before stopping. Sure, this version should do the same (still untested, I don't have a machine with nForce here at the moment). I just wanted to avoid more NIC register accesses, but it's probably a premature optimization. Regards, -- Jindrich Makovicka [-- Attachment #2: forcedeth2.diff --] [-- Type: text/x-patch, Size: 2001 bytes --] --- forcedeth.c.orig 2010-10-07 08:56:55.564511153 +0200 +++ forcedeth.c 2010-10-09 22:34:53.151523511 +0200 @@ -3027,9 +3027,15 @@ { struct fe_priv *np = netdev_priv(dev); u8 __iomem *base = get_hwbase(dev); - u32 addr[2]; - u32 mask[2]; - u32 pff = readl(base + NvRegPacketFilterFlags) & NVREG_PFF_PAUSE_RX; + u32 addr[2], prev_addr[2]; + u32 mask[2], prev_mask[2]; + u32 prev_pff = readl(base + NvRegPacketFilterFlags); + u32 pff = prev_pff & NVREG_PFF_PAUSE_RX; + + prev_addr[0] = readl(base + NvRegMulticastAddrA); + prev_addr[1] = readl(base + NvRegMulticastAddrB); + prev_mask[0] = readl(base + NvRegMulticastMaskA); + prev_mask[1] = readl(base + NvRegMulticastMaskB); memset(addr, 0, sizeof(addr)); memset(mask, 0, sizeof(mask)); @@ -3072,17 +3078,25 @@ } addr[0] |= NVREG_MCASTADDRA_FORCE; pff |= NVREG_PFF_ALWAYS; - spin_lock_irq(&np->lock); - nv_stop_rx(dev); - writel(addr[0], base + NvRegMulticastAddrA); - writel(addr[1], base + NvRegMulticastAddrB); - writel(mask[0], base + NvRegMulticastMaskA); - writel(mask[1], base + NvRegMulticastMaskB); - writel(pff, base + NvRegPacketFilterFlags); - dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n", - dev->name); - nv_start_rx(dev); - spin_unlock_irq(&np->lock); + if (prev_pff != pff + || memcmp(prev_addr, addr, sizeof(prev_addr)) != 0 + || memcmp(prev_mask, mask, sizeof(prev_mask)) != 0) + { + dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n", + dev->name); + spin_lock_irq(&np->lock); + nv_stop_rx(dev); + writel(addr[0], base + NvRegMulticastAddrA); + writel(addr[1], base + NvRegMulticastAddrB); + writel(mask[0], base + NvRegMulticastMaskA); + writel(mask[1], base + NvRegMulticastMaskB); + writel(pff, base + NvRegPacketFilterFlags); + nv_start_rx(dev); + spin_unlock_irq(&np->lock); + } else { + dprintk(KERN_INFO "%s: pff state unchanged - skipping reconfiguration.\n", + dev->name); + } } static void nv_update_pause(struct net_device *dev, u32 pause_flags) ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-10-09 20:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20100812115430.2b5d8683@starbug.prg01.itonis.net>
2010-10-09 11:26 ` Fwd: [PATCH] forcedeth: reconfigure multicast packet filter only when needed Jindřich Makovička
2010-10-09 18:32 ` Stephen Hemminger
2010-10-09 20:46 ` Jindřich Makovička
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox