Page MenuHomeFreeBSD

netgraph/ng_bridge: Add counters for the first link, too.
ClosedPublic

Authored by donner on Mon, Feb 8, 1:28 PM.

Details

Summary

For broadcast, multicast and unknown unicast, the replication loop does
send a copy of the packet to each appropriate link, beside the first
one. So the first one can get the original packet, which saves
additional duplication and free of the original. For this path the
counters are not updated.

Test Plan
# ngctl -f- <<END
mkpeer bridge x link10
mkpeer x eiface link0 ether
mkpeer x eiface link1 ether
mkpeer x eiface link2 ether
END
# ifconfig ngeth0 up
# ifconfig ngeth0 inet 192.168.254.1/30 alias

Assigning an IP address will cause gratious ARP send via broadcast.

# ngctl msg ngeth0:ether getstats 0
Rec'd response "getstats" (4) from "[86b9]:":
Args:   { recvOctets=42 recvPackets=1 recvBroadcast=1 }
# ngctl msg ngeth0:ether getstats 1
Rec'd response "getstats" (4) from "[86b9]:":
Args:   { xmitOctets=42 xmitPackets=1 xmitBroadcasts=1 }
# ngctl msg ngeth0:ether getstats 2
Rec'd response "getstats" (4) from "[86b9]:":
Args:   {}

So the last added hook (which is prepended to the hook list) will be
the first one in traversal and go down the special path: No counters.

After applying the patch:

# ngctl msg ngeth0:ether getstats 0
Rec'd response "getstats" (4) from "[46]:":
Args:   { recvOctets=42 recvPackets=1 recvBroadcast=1 }
# ngctl msg ngeth0:ether getstats 1
Rec'd response "getstats" (4) from "[46]:":
Args:   { xmitOctets=42 xmitPackets=1 xmitBroadcasts=1 }
# ngctl msg ngeth0:ether getstats 2
Rec'd response "getstats" (4) from "[46]:":
Args:   { xmitOctets=42 xmitPackets=1 xmitBroadcasts=1 }

All counters are updated in the same way.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

donner requested review of this revision.Mon, Feb 8, 1:28 PM
kp added a subscriber: kp.

Typo in the commit message: 'approbriate'

The other remark can arguably be better addressed as a separate commit, so this is good to go as far as I'm concerned.

sys/netgraph/ng_bridge.c
864

We've got a very similar code block in the if (!ctx.manycast) case above and in ng-bridge_send_ctx().
It may be worth factoring that off into a separate function. (And when static the compiler will just inline it for you, so it won't even cost much, or any performance)

This revision is now accepted and ready to land.Tue, Feb 9, 9:11 PM
  • Factor out a send and statistic routine
This revision now requires review to proceed.Wed, Feb 10, 8:04 AM

Compiles fine (backported to stable/13), but causes kernel panic

KDB: stack backtrace:
#0 0xffffffff80c56695 at kdb_backtrace+0x65
#1 0xffffffff80c09261 at vpanic+0x181
#2 0xffffffff80c090d3 at panic+0x43
#3 0xffffffff810891a7 at trap_fatal+0x387
#4 0xffffffff810891ff at trap_pfault+0x4f
#5 0xffffffff8108885d at trap+0x27d
#6 0xffffffff8105fd28 at calltrap+0x8
#7 0xffffffff823347ec at ng_apply_item+0x8c
#8 0xffffffff823344b0 at ng_snd_item+0x1d0
#9 0xffffffff82341b92 at ng_eiface_start2+0x172
#10 0xffffffff82334839 at ng_apply_item+0xd9
#11 0xffffffff823344b0 at ng_snd_item+0x1d0
#12 0xffffffff823418b7 at ng_eiface_start+0x37
#13 0xffffffff80d203f9 at if_transmit+0xf9
#14 0xffffffff80d228bb at ether_output_frame+0xab
#15 0xffffffff80d227b8 at ether_output+0x6b8
#16 0xffffffff80d94ed4 at arprequest_internal+0x3b4
#17 0xffffffff80d9572d at arp_ifinit+0x6d

during testing.

  • Fix use after lost reference.

Reverting the counters and the sendout was a promising idea (do not
count, if sending failed), but the mbuf is lost.

This revision is now accepted and ready to land.Wed, Feb 10, 5:41 PM