Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring Rx-placement and Rx-mode #1344

Merged
merged 16 commits into from
May 17, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ examples:
cd examples/kvscheduler/l2 && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/kvscheduler/acl && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/kvscheduler/nat && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/kvscheduler/rxplacement && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/kvscheduler/vpp-l3 && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/kvscheduler/vrf && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/localclient_linux/tap && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/localclient_linux/veth && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
cd examples/localclient_vpp/nat && go build -tags="${GO_BUILD_TAGS}" ${GO_BUILD_ARGS}
Expand Down
248 changes: 124 additions & 124 deletions api/models/vpp/interfaces/interface.pb.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions api/models/vpp/interfaces/interface.proto
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ message Interface {
// <mode> will be used for all queues without explicitly
// selected Rx mode
}
repeated RxMode rx_mode = 10;
repeated RxMode rx_modes = 10;

message RxPlacement {
uint32 queue = 1; // select from interval <0, number-of-queues)
uint32 worker = 2; // select from interval <0, number-of-workers)
bool main_thread = 3; // let the main thread to process the given queue
// - if enabled, value of <worker> is ignored
}
repeated RxPlacement rx_placement = 11;
repeated RxPlacement rx_placements = 11;

oneof link {
SubInterface sub = 100; /* sub-interface configuration */
Expand Down
12 changes: 6 additions & 6 deletions examples/kvscheduler/rxplacement/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,12 @@ func testLocalClientWithScheduler(kvscheduler kvs_api.KVScheduler) {
time.Sleep(time.Second * 10)
fmt.Println("=== CHANGE ===")

myMemif.RxMode[0].Mode = vpp_interfaces.Interface_RxMode_INTERRUPT // change default
myMemif.RxMode = append(myMemif.RxMode, &vpp_interfaces.Interface_RxMode{
myMemif.RxModes[0].Mode = vpp_interfaces.Interface_RxMode_INTERRUPT // change default
myMemif.RxModes = append(myMemif.RxModes, &vpp_interfaces.Interface_RxMode{
Queue: 3,
Mode: vpp_interfaces.Interface_RxMode_POLLING,
})
myMemif.RxPlacement = append(myMemif.RxPlacement, &vpp_interfaces.Interface_RxPlacement{
myMemif.RxPlacements = append(myMemif.RxPlacements, &vpp_interfaces.Interface_RxPlacement{
Queue: 3,
MainThread: true,
Worker: 100, // ignored
Expand All @@ -194,7 +194,7 @@ func testLocalClientWithScheduler(kvscheduler kvs_api.KVScheduler) {

myMemif.GetMemif().RxQueues = 5
myMemif.GetMemif().TxQueues = 5
myMemif.RxPlacement = append(myMemif.RxPlacement, &vpp_interfaces.Interface_RxPlacement{
myMemif.RxPlacements = append(myMemif.RxPlacements, &vpp_interfaces.Interface_RxPlacement{
Queue: 4,
MainThread: true,
})
Expand All @@ -221,7 +221,7 @@ var (
Enabled: true,
IpAddresses: []string{"192.168.1.1/24"},

RxPlacement: []*vpp_interfaces.Interface_RxPlacement{
RxPlacements: []*vpp_interfaces.Interface_RxPlacement{
{
Queue: 0,
Worker: 0,
Expand All @@ -236,7 +236,7 @@ var (
},
},

RxMode: []*vpp_interfaces.Interface_RxMode{
RxModes: []*vpp_interfaces.Interface_RxMode{
{
DefaultMode: true,
Mode: vpp_interfaces.Interface_RxMode_POLLING,
Expand Down
6 changes: 5 additions & 1 deletion plugins/kvscheduler/internal/registry/registry_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

. "github.com/ligato/vpp-agent/plugins/kvscheduler/api"
"github.com/ligato/vpp-agent/plugins/kvscheduler/internal/utils"
"fmt"
)

const (
Expand Down Expand Up @@ -111,8 +112,11 @@ func (reg *registry) GetDescriptorForKey(key string) *KVDescriptor {
var keyDescriptor *KVDescriptor
for _, descriptor := range reg.descriptors {
if descriptor.KeySelector(key) {
if keyDescriptor != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was just thinking if this is actually not gonna make GetDescriptorForKey more expensive performance-wise. Can you run the perf tests with 20k to compare difference?

Copy link
Collaborator Author

@milanlenco milanlenco May 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It varies quite a bit on my system, but there doesn't seem to be measurable drop in the performance:

BEFORE:

level=info msg="Client #0 done, 20000 tunnels took 1m26.308s" loc="grpc-perf/main.go(302)" logger=grpc-stress-test-client
level=info msg="All clients done, took: 1m16.7757s" loc="grpc-perf/main.go(219)" logger=grpc-stress-test-client
level=info msg="All clients done, took: 1m15.096s" loc="grpc-perf/main.go(219)" logger=grpc-stress-test-client

AFTER:

level=info msg="All clients done, took: 1m5.0734s" loc="grpc-perf/main.go(219)" logger=grpc-stress-test-client
level=info msg="All clients done, took: 1m18.1164s" loc="grpc-perf/main.go(219)" logger=grpc-stress-test-client
level=info msg="All clients done, took: 1m12.1412s" loc="grpc-perf/main.go(219)" logger=grpc-stress-test-client

This lookup is cached (for the last 500 different keys), so effectively each distinct key goes through the cycle only once and the number of descriptors isn't that significant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, merging then.

panic(fmt.Sprintf("key %s is selected by both %s and %s descriptors",
key, keyDescriptor.Name, descriptor.Name))
}
keyDescriptor = descriptor
break
}
}
// add entry to cache
Expand Down
16 changes: 8 additions & 8 deletions plugins/vpp/ifplugin/descriptor/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,9 +434,9 @@ func (d *InterfaceDescriptor) Validate(key string, intf *interfaces.Interface) e
}

// validate Rx Placement before it gets derived out
for i, rxPlacement1 := range intf.GetRxPlacement() {
for j := i + 1; j < len(intf.GetRxPlacement()); j++ {
rxPlacement2 := intf.GetRxPlacement()[j]
for i, rxPlacement1 := range intf.GetRxPlacements() {
for j := i + 1; j < len(intf.GetRxPlacements()); j++ {
rxPlacement2 := intf.GetRxPlacements()[j]
if rxPlacement1.Queue == rxPlacement2.Queue {
return kvs.NewInvalidValueError(ErrRedefinedRxPlacement,
fmt.Sprintf("rx_placement[.queue=%d]", rxPlacement1.Queue))
Expand Down Expand Up @@ -603,19 +603,19 @@ func (d *InterfaceDescriptor) DerivedValues(key string, intf *interfaces.Interfa
}

// Rx mode
if len(intf.GetRxMode()) > 0 {
if len(intf.GetRxModes()) > 0 {
derValues = append(derValues, kvs.KeyValuePair{
Key: interfaces.RxModeKey(intf.GetName()),
Value: &interfaces.Interface{
Name: intf.GetName(),
Type: intf.GetType(),
RxMode: intf.GetRxMode(),
Name: intf.GetName(),
Type: intf.GetType(),
RxModes: intf.GetRxModes(),
},
})
}

// Rx placement
for _, rxPlacement := range intf.GetRxPlacement() {
for _, rxPlacement := range intf.GetRxPlacements() {
derValues = append(derValues, kvs.KeyValuePair{
Key: interfaces.RxPlacementKey(intf.GetName(), rxPlacement.GetQueue()),
Value: rxPlacement,
Expand Down
10 changes: 5 additions & 5 deletions plugins/vpp/ifplugin/descriptor/interface_crud.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,8 @@ func (d *InterfaceDescriptor) Retrieve(correlate []adapter.InterfaceKVWithMetada
}

// remove rx-placement entries for queues with configuration not defined by NB
rxPlacementDump := intf.Interface.GetRxPlacement()
rxPlacementCfg := expCfg.GetRxPlacement()
rxPlacementDump := intf.Interface.GetRxPlacements()
rxPlacementCfg := expCfg.GetRxPlacements()
for i := 0; i < len(rxPlacementDump); {
queue := rxPlacementDump[i].Queue
found := false
Expand All @@ -438,11 +438,11 @@ func (d *InterfaceDescriptor) Retrieve(correlate []adapter.InterfaceKVWithMetada
rxPlacementDump = append(rxPlacementDump[:i], rxPlacementDump[i+1:]...)
}
}
intf.Interface.RxPlacement = rxPlacementDump
intf.Interface.RxPlacements = rxPlacementDump

// remove rx-mode from the dump if it is not configured by NB
if len(expCfg.GetRxMode()) == 0 {
intf.Interface.RxMode = []*interfaces.Interface_RxMode{}
if len(expCfg.GetRxModes()) == 0 {
intf.Interface.RxModes = []*interfaces.Interface_RxMode{}
}
}

Expand Down
18 changes: 9 additions & 9 deletions plugins/vpp/ifplugin/descriptor/rx_mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (d *RxModeDescriptor) EquivalentRxMode(key string, oldIntf, newIntf *interf
}
}
// compare queue-specific RX modes
for _, rxMode := range oldIntf.GetRxMode() {
for _, rxMode := range oldIntf.GetRxModes() {
if rxMode.DefaultMode {
continue
}
Expand All @@ -113,7 +113,7 @@ func (d *RxModeDescriptor) EquivalentRxMode(key string, oldIntf, newIntf *interf
return false
}
}
for _, rxMode := range newIntf.GetRxMode() {
for _, rxMode := range newIntf.GetRxModes() {
if rxMode.DefaultMode {
continue
}
Expand All @@ -128,16 +128,16 @@ func (d *RxModeDescriptor) EquivalentRxMode(key string, oldIntf, newIntf *interf

// Validate validates Rx mode configuration.
func (d *RxModeDescriptor) Validate(key string, ifaceWithRxMode *interfaces.Interface) error {
for i, rxMode1 := range ifaceWithRxMode.GetRxMode() {
for i, rxMode1 := range ifaceWithRxMode.GetRxModes() {
if rxMode1.Mode == interfaces.Interface_RxMode_UNKNOWN {
if rxMode1.DefaultMode {
return kvs.NewInvalidValueError(ErrUndefinedRxMode,"rx_mode[default]")
}
return kvs.NewInvalidValueError(ErrUndefinedRxMode,
fmt.Sprintf("rx_mode[.queue=%d]", rxMode1.Queue))
}
for j := i + 1; j < len(ifaceWithRxMode.GetRxMode()); j++ {
rxMode2 := ifaceWithRxMode.GetRxMode()[j]
for j := i + 1; j < len(ifaceWithRxMode.GetRxModes()); j++ {
rxMode2 := ifaceWithRxMode.GetRxModes()[j]
if rxMode1.DefaultMode != rxMode2.DefaultMode {
continue
}
Expand All @@ -152,7 +152,7 @@ func (d *RxModeDescriptor) Validate(key string, ifaceWithRxMode *interfaces.Inte
}

if ifaceWithRxMode.GetType() == interfaces.Interface_DPDK {
for _, rxMode := range ifaceWithRxMode.GetRxMode() {
for _, rxMode := range ifaceWithRxMode.GetRxModes() {
mode := normalizeRxMode(rxMode.Mode, ifaceWithRxMode)
if mode != interfaces.Interface_RxMode_POLLING {
if rxMode.DefaultMode {
Expand Down Expand Up @@ -235,7 +235,7 @@ func (d *RxModeDescriptor) configureRxMode(iface *interfaces.Interface, op kvs.T
}

// configure per-queue RX mode
for _, rxMode := range iface.GetRxMode() {
for _, rxMode := range iface.GetRxModes() {
if rxMode.DefaultMode || rxMode.Mode == defRxMode {
continue
}
Expand Down Expand Up @@ -264,7 +264,7 @@ func (d *RxModeDescriptor) Dependencies(key string, ifaceWithRxMode *interfaces.

// getDefaultRxMode reads default RX mode from the interface configuration.
func getDefaultRxMode(iface *interfaces.Interface) (rxMode interfaces.Interface_RxMode_Type) {
for _, rxMode := range iface.GetRxMode() {
for _, rxMode := range iface.GetRxModes() {
if rxMode.DefaultMode {
return normalizeRxMode(rxMode.Mode, iface)
}
Expand All @@ -274,7 +274,7 @@ func getDefaultRxMode(iface *interfaces.Interface) (rxMode interfaces.Interface_

// getQueueRxMode reads RX mode for the given queue from the interface configuration.
func getQueueRxMode(queue uint32, iface *interfaces.Interface) (mode interfaces.Interface_RxMode_Type) {
for _, rxMode := range iface.GetRxMode() {
for _, rxMode := range iface.GetRxModes() {
if rxMode.DefaultMode {
mode = rxMode.Mode
continue // keep looking for a queue-specific RX mode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ func (h *InterfaceVppHandler) dumpRxPlacement(ifs map[uint32]*vppcalls.Interface
continue
}

ifData.Interface.RxMode = append(ifData.Interface.RxMode,
ifData.Interface.RxModes = append(ifData.Interface.RxModes,
&interfaces.Interface_RxMode{
Queue: rxDetails.QueueID,
Mode: getRxModeType(rxDetails.Mode),
Expand All @@ -804,7 +804,7 @@ func (h *InterfaceVppHandler) dumpRxPlacement(ifs map[uint32]*vppcalls.Interface
if rxDetails.WorkerID > 0 {
worker = rxDetails.WorkerID - 1
}
ifData.Interface.RxPlacement = append(ifData.Interface.RxPlacement,
ifData.Interface.RxPlacements = append(ifData.Interface.RxPlacements,
&interfaces.Interface_RxPlacement{
Queue: rxDetails.QueueID,
Worker: worker,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ func TestDumpInterfacesRxPlacement(t *testing.T) {
Expect(intface.GetMemif().Mode).To(Equal(interfaces2.MemifLink_IP))
Expect(intface.GetMemif().Master).To(BeFalse())

rxMode := intface.GetRxMode()
rxMode := intface.GetRxModes()
Expect(rxMode).To(HaveLen(3))
Expect(rxMode[0].Queue).To(BeEquivalentTo(0))
Expect(rxMode[0].Mode).To(BeEquivalentTo(interfaces2.Interface_RxMode_ADAPTIVE))
Expand All @@ -503,7 +503,7 @@ func TestDumpInterfacesRxPlacement(t *testing.T) {
Expect(rxMode[2].Queue).To(BeEquivalentTo(2))
Expect(rxMode[2].Mode).To(BeEquivalentTo(interfaces2.Interface_RxMode_POLLING))

rxPlacement := intface.GetRxPlacement()
rxPlacement := intface.GetRxPlacements()
Expect(rxPlacement).To(HaveLen(3))
Expect(rxPlacement[0].Queue).To(BeEquivalentTo(0))
Expect(rxPlacement[0].MainThread).To(BeTrue())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ func (h *InterfaceVppHandler) dumpRxPlacement(ifs map[uint32]*vppcalls.Interface
continue
}

ifData.Interface.RxMode = append(ifData.Interface.RxMode,
ifData.Interface.RxModes = append(ifData.Interface.RxModes,
&interfaces.Interface_RxMode{
Queue: rxDetails.QueueID,
Mode: getRxModeType(rxDetails.Mode),
Expand All @@ -820,7 +820,7 @@ func (h *InterfaceVppHandler) dumpRxPlacement(ifs map[uint32]*vppcalls.Interface
if rxDetails.WorkerID > 0 {
worker = rxDetails.WorkerID - 1
}
ifData.Interface.RxPlacement = append(ifData.Interface.RxPlacement,
ifData.Interface.RxPlacements = append(ifData.Interface.RxPlacements,
&interfaces.Interface_RxPlacement{
Queue: rxDetails.QueueID,
Worker: worker,
Expand Down
Loading