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

Multicast statistics implementation #3449

Merged
merged 1 commit into from
Jun 14, 2022
Merged

Conversation

ceclinux
Copy link
Contributor

@ceclinux ceclinux commented Mar 15, 2022

This PR added multicast statistics support for the following cases:

  1. Supplements current networkpolicy statistics implementation by parsing multicast related flows, which can be displayed as kubectl get antreanetworkpolicystats multicast-networkpolicy-name
  2. Add a node-level antctl command antctl get multicaststats, showing inbound and outbound packet count for each pod interface.
  3. Add an extra kubectl get multicastgroupmembers command. This command shows which pods have joined multicast group for the whole cluster.
  • rebase updated bin's patch
  • add tests

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2022

Codecov Report

Merging #3449 (8284475) into main (7817152) will decrease coverage by 0.02%.
The diff coverage is 27.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3449      +/-   ##
==========================================
- Coverage   64.48%   64.46%   -0.03%     
==========================================
  Files         291      293       +2     
  Lines       42795    43224     +429     
==========================================
+ Hits        27598    27864     +266     
- Misses      12974    13130     +156     
- Partials     2223     2230       +7     
Flag Coverage Δ
e2e-tests 44.74% <12.42%> (?)
kind-e2e-tests 50.57% <12.42%> (-0.42%) ⬇️
unit-tests 44.56% <26.43%> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../agent/apiserver/handlers/networkpolicy/handler.go 48.97% <0.00%> (ø)
...g/agent/apiserver/handlers/podinterface/handler.go 70.83% <ø> (ø)
pkg/agent/multicast/mcast_controller.go 51.93% <0.00%> (-6.16%) ⬇️
pkg/agent/openflow/multicast.go 0.00% <0.00%> (ø)
pkg/agent/openflow/pipeline.go 72.94% <ø> (+0.22%) ⬆️
pkg/antctl/antctl.go 50.00% <ø> (ø)
pkg/controller/stats/aggregator.go 68.95% <0.00%> (-12.10%) ⬇️
pkg/querier/querier.go 55.00% <ø> (ø)
...kg/apiserver/registry/stats/multicastgroup/rest.go 11.59% <11.59%> (ø)
pkg/agent/stats/collector.go 74.29% <15.38%> (-20.87%) ⬇️
... and 35 more

@ceclinux
Copy link
Contributor Author

/test-multicast-e2e

3 similar comments
@ceclinux
Copy link
Contributor Author

/test-multicast-e2e

@ceclinux
Copy link
Contributor Author

/test-multicast-e2e

@ceclinux
Copy link
Contributor Author

/test-multicast-e2e

@ceclinux
Copy link
Contributor Author

ceclinux commented Apr 6, 2022

/test-multicast-e2e

@ceclinux ceclinux force-pushed the multicast-stats branch 2 times, most recently from 9dcd1a2 to 0b6f728 Compare April 7, 2022 03:15
@ceclinux
Copy link
Contributor Author

ceclinux commented Apr 7, 2022

/test-multicast-e2e

1 similar comment
@ceclinux
Copy link
Contributor Author

ceclinux commented Apr 7, 2022

/test-multicast-e2e

@ceclinux ceclinux force-pushed the multicast-stats branch 11 times, most recently from 1ecf91c to 7d3bb79 Compare April 12, 2022 12:16
@tnqn tnqn added this to the Antrea v1.7 release milestone May 13, 2022
@tnqn tnqn added action/release-note Indicates a PR that should be included in release notes. kind/feature Categorizes issue or PR as related to a new feature. labels May 13, 2022
@ceclinux ceclinux changed the title [WIP Multicast] Multicast statistics implementation Multicast statistics implementation May 13, 2022
@ceclinux ceclinux force-pushed the multicast-stats branch 8 times, most recently from 1fb0f2b to d2153dd Compare June 12, 2022 23:40
@@ -99,6 +100,46 @@ func (f *featureMulticast) multicastOutputFlow(cookieID uint64) binding.Flow {
Done()
}

func (f *featureMulticast) multicastSkipIGMPMetricFlows() []binding.Flow {
return []binding.Flow{
Copy link
Contributor

Choose a reason for hiding this comment

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

Use this format?

cookieID := f.cookieAllocator.Request(f.category).Raw()
var flows []binding.Flow
for _, t := range []*Table{MulticastIngressPodMetricTable,  MulticastEgressPodMetricTable} {
     flows = append(flows, t.ofTable.BuildFlow(priorityHigh).
			Cookie(cookieID).
			MatchProtocol(binding.ProtocolIGMP).
			Action().NextTable().
			Done())
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Cookie(f.cookieAllocator.Request(f.category).Raw()).
MatchProtocol(ipProtocol).
MatchSrcIP(podIP).
MatchDstIPNet(*types.McastCIDR).
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it looks not necessary.

wenyingd
wenyingd previously approved these changes Jun 13, 2022
Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/agent/openflow/client.go Show resolved Hide resolved
params: []flagInfo{
{
name: "name",
usage: "Retrieve Pod interface by name. If present, Namespace must be provided.",
Copy link
Member

Choose a reason for hiding this comment

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

wrong usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

},
{
name: "namespace",
usage: "Get Pod interfaces from specific Namespace",
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

for i, pod := range stats.Pods {
podNameSlice[i] = k8s.NamespacedName(pod.Namespace, pod.Name)
}
return []interface{}{stats.Group, strings.Join(podNameSlice, ", ")}, nil
Copy link
Member

Choose a reason for hiding this comment

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

then we should cut it and append a number and ... like other commands since there will be likely some Pods in a Group?

for _, mcastGroupInfo := range summary.Multicast {
group := mcastGroupInfo.Group
reportedGroups.Insert(group)
a.groupNodePodsMapMutex.Lock()
Copy link
Member

Choose a reason for hiding this comment

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

acquring lock is not free, seems not worth to lock and unlock repeatedly here to exclude a few code from critical area, which may take more cost

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@ceclinux ceclinux force-pushed the multicast-stats branch 3 times, most recently from 36cd78e to 1169210 Compare June 13, 2022 13:02
This PR added multicast statistics support for the following cases:

- Supplements current networkpolicy statistics implementation by parsing
  multicast related flows, which can be displayed as
  kubectl get antreanetworkpolicystats multicast-networkpolicy-name.

- Add a node-level antctl command antctl get podmulticaststats,
  showing inbound and outbound packet count for each pod interface.

- Add an extra kubectl get multicastgroups command.
  This command shows which pods have joined multicast group
  for the whole cluster.

Signed-off-by: ceclinux <src655@gmail.com>
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented Jun 13, 2022

/test-all
/test-multicast-e2e

@tnqn
Copy link
Member

tnqn commented Jun 14, 2022

/test-multicast-e2e
/test-ipv6-e2e
/test-ipv6-only-e2e

@tnqn
Copy link
Member

tnqn commented Jun 14, 2022

/test-multicast-e2e

1 similar comment
@tnqn
Copy link
Member

tnqn commented Jun 14, 2022

/test-multicast-e2e

@tnqn tnqn merged commit 6782d71 into antrea-io:main Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants