tests: Remove inet6 jumbogram tests
Needs ReviewPublic
Actions

Authored by pouria on Mon, Mar 16, 10:41 PM.

Details

Reviewers

glebius
thj
tuexen
jtl

Group Reviewers

network

Summary

Jumbogram support has been removed in
069a67374ed9. Therefore, its tests should be remove too.
Already disabled so, no functional change.

The problem is fragment test numbering now starts from 2.

Test Plan

no functional change.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 71436
Build 68319: arc lint + arc unit

Event Timeline

pouria created this revision.Mon, Mar 16, 10:41 PM

Herald added subscribers: melifaro, ae, asomers, imp. · View Herald TranscriptMon, Mar 16, 10:41 PM

pouria requested review of this revision.Mon, Mar 16, 10:41 PM

Harbormaster completed remote builds in B71436: Diff 173775.Mon, Mar 16, 10:41 PM

I have not looked into the tests. Since the Jumbogram feature has been removed in the inet6 stack, shall the relevant tests be updated to reflect the removal but not removing the tests ?

In D55887#1278653, @zlei wrote:

I have not looked into the tests. Since the Jumbogram feature has been removed in the inet6 stack, shall the relevant tests be updated to reflect the removal but not removing the tests ?

Could you please explain more?
Is there any other jumbogram tests that I missed or you mean renumbering the fragment tests?
If it's the latter, I don't like to mix that one with this change because it would affect the git blame result of all the other fragment tests.
I'll do that in a separate revision and then I can add that commit into .git-blame-ignore-revs to avoid its side-effects.

In D55887#1278756, @pouria wrote:

In D55887#1278653, @zlei wrote:

I have not looked into the tests. Since the Jumbogram feature has been removed in the inet6 stack, shall the relevant tests be updated to reflect the removal but not removing the tests ?

Could you please explain more?
Is there any other jumbogram tests that I missed or you mean renumbering the fragment tests?
If it's the latter, I don't like to mix that one with this change because it would affect the git blame result of all the other fragment tests.
I'll do that in a separate revision and then I can add that commit into .git-blame-ignore-revs to avoid its side-effects.

It is generally good practices to increase the code coverage, but not decrease, if possible.

For this case, quoted from D19960,

We no not support Jumbo Payloads so report an error

probably in the test case ip6s_badoptions and / or ICMP6 error should be checked if the kernel is injected with an Jumbogram ?

I think we should probably have tests check we do the correct things when we get a jumbogram. If these tests aren't run they don't really help with that.

I don't think the numbering gap is a problem, I'd leave the others alone.

Do you have any history for why these are disabled? I they are from the initial netflix work when the frag vulnerabilities came up?

bms added a subscriber: bms.Wed, Mar 18, 10:01 AM